Re: [PATCH RFC v2 00/37] drm/connector: Create HDMI Connector infrastructure

From: Hans Verkuil
Date: Thu Sep 21 2023 - 18:21:03 EST

Next message: Stefan Roesch: "[PATCH v3 0/2] mm/ksm: add fork-exec support for prctl"
Previous message: Kalle Valo: "Re: [PATCH] wifi: ipw2x00: Annotate struct libipw_txb with __counted_by"
In reply to: Maxime Ripard: "[PATCH RFC v2 37/37] drm/sun4i: hdmi: Switch to HDMI connector"
Next in thread: Maxime Ripard: "Re: [PATCH RFC v2 00/37] drm/connector: Create HDMI Connector infrastructure"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 20/09/2023 16:35, Maxime Ripard wrote:
> Hi,
>
> Here's a series that creates a subclass of drm_connector specifically
> targeted at HDMI controllers.
>
> The idea behind this series came from a recent discussion on IRC during
> which we discussed infoframes generation of i915 vs everything else.
>
> Infoframes generation code still requires some decent boilerplate, with
> each driver doing some variation of it.
>
> In parallel, while working on vc4, we ended up converting a lot of i915
> logic (mostly around format / bpc selection, and scrambler setup) to
> apply on top of a driver that relies only on helpers.
>
> While currently sitting in the vc4 driver, none of that logic actually
> relies on any driver or hardware-specific behaviour.
>
> The only missing piece to make it shareable are a bunch of extra
> variables stored in a state (current bpc, format, RGB range selection,
> etc.).
>
> The initial implementation was relying on some generic subclass of
> drm_connector to address HDMI connectors, with a bunch of helpers that
> will take care of all the "HDMI Spec" related code. Scrambler setup is
> missing at the moment but can easily be plugged in.
>
> The feedback was that creating a connector subclass like was done for
> writeback would prevent the adoption of those helpers since it couldn't
> be used in all situations (like when the connector driver can implement
> multiple output) and required more churn to cast between the
> drm_connector and its subclass. The decision was thus to provide a set
> of helper and to store the required variables in drm_connector and
> drm_connector_state. This what has been implemented now.
>
> Hans Verkuil also expressed interest in implementing a mechanism in v4l2
> to retrieve infoframes from HDMI receiver and implementing an
> infoframe-decode tool.

I'd love to get started on that, but...

>
> This series thus leverages the infoframe generation code to expose it
> through debugfs.
>
> This entire series is only build-tested at the moment. Let me know what
> you think,

...trying this series on my RPi4 gives me this during boot:

[ 2.361239] vc4-drm gpu: bound fe400000.hvs (ops 0xffff800080cac6f8)
[ 2.367834] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000090
[ 2.376748] Mem abort info:
[ 2.379570] ESR = 0x0000000096000044
[ 2.383367] EC = 0x25: DABT (current EL), IL = 32 bits
[ 2.388748] SET = 0, FnV = 0
[ 2.391835] EA = 0, S1PTW = 0
[ 2.395011] FSC = 0x04: level 0 translation fault
[ 2.399951] Data abort info:
[ 2.402864] ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
[ 2.408420] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[ 2.413536] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 2.418916] [0000000000000090] user address but active_mm is swapper
[ 2.425353] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
[ 2.431700] Modules linked in:
[ 2.434791] CPU: 2 PID: 55 Comm: kworker/u8:3 Not tainted 6.6.0-rc1-hdmi-dbg #245
[ 2.442372] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
[ 2.448278] Workqueue: events_unbound deferred_probe_work_func
[ 2.454193] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2.461245] pc : drm_connector_attach_max_bpc_property+0x48/0x90
[ 2.467332] lr : drm_connector_attach_max_bpc_property+0x3c/0x90
[ 2.473415] sp : ffff800081d038b0
[ 2.476766] x29: ffff800081d038b0 x28: 0000000000000000 x27: ffff0001041968c0
[ 2.483999] x26: 0000000000000000 x25: ffff00010339d558 x24: ffff000103399000
[ 2.491231] x23: ffff800080caa3e8 x22: ffff800080e96a20 x21: 000000000000000c
[ 2.498463] x20: 000000000000000c x19: ffff00010339d558 x18: ffffffffffffffff
[ 2.505694] x17: ffff0001008e7650 x16: ffff800080d55500 x15: ffffffffffffffff
[ 2.512926] x14: ffff000105dda209 x13: 0000000000000006 x12: 0000000000000001
[ 2.520158] x11: 0101010101010101 x10: ffff00027effe219 x9 : 0000000000000001
[ 2.527389] x8 : ffff000105db8ad4 x7 : 00000000c0c0c0c0 x6 : 00000000c0c0c0c0
[ 2.534620] x5 : 0000000000000000 x4 : ffff00010339d728 x3 : ffff00010339d728
[ 2.541852] x2 : 000000000000000c x1 : 0000000000000000 x0 : 0000000000000000
[ 2.549083] Call trace:
[ 2.551554] drm_connector_attach_max_bpc_property+0x48/0x90
[ 2.557285] drmm_connector_hdmi_init+0x114/0x14c
[ 2.562048] vc4_hdmi_bind+0x320/0xa40
[ 2.565842] component_bind_all+0x114/0x23c
[ 2.570077] vc4_drm_bind+0x148/0x2c0
[ 2.573784] try_to_bring_up_aggregate_device+0x168/0x1d4
[ 2.579253] __component_add+0xa4/0x16c
[ 2.583136] component_add+0x14/0x20
[ 2.586754] vc4_hdmi_dev_probe+0x1c/0x28
[ 2.590815] platform_probe+0x68/0xc4
[ 2.594522] really_probe+0x148/0x2b0
[ 2.598228] __driver_probe_device+0x78/0x12c
[ 2.602638] driver_probe_device+0xd8/0x15c
[ 2.606873] __device_attach_driver+0xb8/0x134
[ 2.611372] bus_for_each_drv+0x80/0xdc
[ 2.615254] __device_attach+0x9c/0x188
[ 2.619136] device_initial_probe+0x14/0x20
[ 2.623371] bus_probe_device+0xac/0xb0
[ 2.627253] deferred_probe_work_func+0x88/0xc0
[ 2.631839] process_one_work+0x138/0x244
[ 2.635899] worker_thread+0x320/0x438
[ 2.639692] kthread+0x10c/0x110
[ 2.642957] ret_from_fork+0x10/0x20
[ 2.646576] Code: 94005f8d 12001e94 f9427e61 52800000 (39024034)
[ 2.652745] ---[ end trace 0000000000000000 ]---

Regards,

Hans

> Maxime
>
> Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
> Changes in v2:
> - Change from a subclass to a set of helpers for drm_connector and
> drm_connector state
> - Don't assume that all drivers support RGB, YUV420 and YUV422 but make
> them provide a bitfield instead.
> - Don't assume that all drivers support the Broadcast RGB property but
> make them call the registration helper.
> - Document the Broacast RGB property
> - Convert the inno_hdmi and sun4i_hdmi driver.
> - Link to v1: https://lore.kernel.org/r/20230814-kms-hdmi-connector-state-v1-0-048054df3654@xxxxxxxxxx
>
> ---
> Maxime Ripard (37):
> drm/connector: Introduce an HDMI connector
> drm/connector: hdmi: Create a custom state
> drm/connector: hdmi: Add Broadcast RGB property
> drm/connector: hdmi: Add helper to get the RGB range
> drm/connector: hdmi: Add output BPC to the connector state
> drm/connector: hdmi: Add support for output format
> drm/connector: hdmi: Add HDMI compute clock helper
> drm/connector: hdmi: Calculate TMDS character rate
> drm/connector: hdmi: Add custom hook to filter TMDS character rate
> drm/connector: hdmi: Compute bpc and format automatically
> drm/connector: hdmi: Add Infoframes generation
> drm/connector: hdmi: Create Infoframe DebugFS entries
> drm/vc4: hdmi: Create destroy state implementation
> drm/vc4: hdmi: Switch to HDMI connector
> drm/rockchip: inno_hdmi: Remove useless mode_fixup
> drm/rockchip: inno_hdmi: Remove useless copy of drm_display_mode
> drm/rockchip: inno_hdmi: Switch encoder hooks to atomic
> drm/rockchip: inno_hdmi: Get rid of mode_set
> drm/rockchip: inno_hdmi: no need to store vic
> drm/rockchip: inno_hdmi: Remove unneeded has audio flag
> drm/rockchip: inno_hdmi: Remove useless input format
> drm/rockchip: inno_hdmi: Remove useless output format
> drm/rockchip: inno_hdmi: Remove useless colorimetry
> drm/rockchip: inno_hdmi: Remove useless enum
> drm/rockchip: inno_hdmi: Remove tmds rate from structure
> drm/rockchip: inno_hdmi: Remove useless coeff_csc matrix
> drm/rockchip: inno_hdmi: Remove useless mode_valid
> drm/rockchip: inno_hdmi: Move infoframe disable to separate function
> drm/rockchip: inno_hdmi: Create mask retrieval functions
> drm/rockchip: inno_hdmi: Switch to infoframe type
> drm/rockchip: inno_hdmi: Remove unused drm device pointer
> drm/rockchip: inno_hdmi: Switch to HDMI connector
> drm/sun4i: hdmi: Convert encoder to atomic
> drm/sun4i: hdmi: Move mode_set into enable
> drm/sun4i: hdmi: Switch to container_of_const
> drm/sun4i: hdmi: Consolidate atomic_check and mode_valid
> drm/sun4i: hdmi: Switch to HDMI connector
>
> Documentation/gpu/kms-properties.csv | 1 -
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c | 10 +
> drivers/gpu/drm/drm_atomic_state_helper.c | 634 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 +
> drivers/gpu/drm/drm_connector.c | 196 +++++++++
> drivers/gpu/drm/drm_debugfs.c | 110 ++++++
> drivers/gpu/drm/rockchip/inno_hdmi.c | 409 +++++++------------
> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 203 +++++-----
> drivers/gpu/drm/vc4/vc4_hdmi.c | 624 ++++-------------------------
> drivers/gpu/drm/vc4/vc4_hdmi.h | 44 +--
> drivers/gpu/drm/vc4/vc4_hdmi_phy.c | 6 +-
> include/drm/drm_atomic_state_helper.h | 15 +
> include/drm/drm_connector.h | 245 ++++++++++++
> 14 files changed, 1557 insertions(+), 945 deletions(-)
> ---
> base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> change-id: 20230814-kms-hdmi-connector-state-616787e67927
>
> Best regards,

Next message: Stefan Roesch: "[PATCH v3 0/2] mm/ksm: add fork-exec support for prctl"
Previous message: Kalle Valo: "Re: [PATCH] wifi: ipw2x00: Annotate struct libipw_txb with __counted_by"
In reply to: Maxime Ripard: "[PATCH RFC v2 37/37] drm/sun4i: hdmi: Switch to HDMI connector"
Next in thread: Maxime Ripard: "Re: [PATCH RFC v2 00/37] drm/connector: Create HDMI Connector infrastructure"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]