[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6qouaxhcwr4rxva7vgq7yy274imiufdtp3ae4mmirzt7hycxc6@4g26bzmhc37h>
Date: Fri, 5 Sep 2025 17:32:42 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Andy Yan <andyshrk@....com>
Cc: heiko@...ech.de, mripard@...nel.org, neil.armstrong@...aro.org,
andrzej.hajda@...el.com, jernej.skrabec@...il.com, jonas@...boo.se,
Laurent.pinchart@...asonboard.com, maarten.lankhorst@...ux.intel.com,
rfoss@...nel.org, simona@...ll.ch, tzimmermann@...e.de,
knaerzche@...il.com, devicetree@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH v7 1/2] drm/rockchip: inno-hdmi: Convert to drm bridge
On Fri, Sep 05, 2025 at 08:40:31AM +0800, Andy Yan wrote:
>
> Hello Dmitry,
>
> At 2025-09-05 08:05:06, "Dmitry Baryshkov" <dmitry.baryshkov@....qualcomm.com> wrote:
> >On Wed, Sep 03, 2025 at 07:07:38PM +0800, Andy Yan wrote:
> >> From: Andy Yan <andy.yan@...k-chips.com>
> >>
> >> Convert it to drm bridge driver, it will be convenient for us to
> >> migrate the connector part to the display driver later.
> >>
> >> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
> >>
> >> ---
> >>
> >> Changes in v7:
> >> - Rebase on latest drm-misc-next
> >>
> >> Changes in v6:
> >> - Rebase on latest drm-misc-next
> >> - Link to V5: https://lore.kernel.org/linux-rockchip/20250512124615.2848731-1-andyshrk@163.com/
> >>
> >> Changes in v5:
> >> - Split cleanup code to seperate patch
> >> - Switch to devm_drm_bridge_alloc() API
> >> - Link to V4: https://lore.kernel.org/linux-rockchip/20250422070455.432666-1-andyshrk@163.com/
> >>
> >> Changes in v4:
> >> - Do not store colorimetry within inno_hdmi struct
> >> - Link to V3: https://lore.kernel.org/linux-rockchip/20250402123150.238234-1-andyshrk@163.com/
> >>
> >> Changes in v3:
> >> - First included in v3
> >> - Link to V2: https://lore.kernel.org/dri-devel/20250325132944.171111-1-andyshrk@163.com/
> >>
> >> drivers/gpu/drm/bridge/Kconfig | 7 +
> >> drivers/gpu/drm/bridge/Makefile | 1 +
> >> .../inno_hdmi.c => bridge/inno-hdmi.c} | 502 +++++-------------
> >> drivers/gpu/drm/rockchip/Kconfig | 1 +
> >> drivers/gpu/drm/rockchip/Makefile | 2 +-
> >> drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c | 188 +++++++
> >> include/drm/bridge/inno_hdmi.h | 33 ++
> >> 7 files changed, 366 insertions(+), 368 deletions(-)
> >> rename drivers/gpu/drm/{rockchip/inno_hdmi.c => bridge/inno-hdmi.c} (69%)
> >> create mode 100644 drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c
> >> create mode 100644 include/drm/bridge/inno_hdmi.h
> >>
> >> @@ -637,14 +584,13 @@ static void inno_hdmi_init_hw(struct inno_hdmi *hdmi)
> >> hdmi_modb(hdmi, HDMI_STATUS, m_MASK_INT_HOTPLUG, v_MASK_INT_HOTPLUG(1));
> >> }
> >>
> >> -static int inno_hdmi_disable_frame(struct drm_connector *connector,
> >> - enum hdmi_infoframe_type type)
> >> +static int inno_hdmi_bridge_clear_infoframe(struct drm_bridge *bridge,
> >> + enum hdmi_infoframe_type type)
> >> {
> >> - struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
> >> + struct inno_hdmi *hdmi = bridge_to_inno_hdmi(bridge);
> >>
> >> if (type != HDMI_INFOFRAME_TYPE_AVI) {
> >> - drm_err(connector->dev,
> >> - "Unsupported infoframe type: %u\n", type);
> >> + drm_err(bridge->dev, "Unsupported infoframe type: %u\n", type);
> >> return 0;
> >> }
> >>
> >> @@ -653,20 +599,19 @@ static int inno_hdmi_disable_frame(struct drm_connector *connector,
> >> return 0;
> >> }
> >>
> >> -static int inno_hdmi_upload_frame(struct drm_connector *connector,
> >> - enum hdmi_infoframe_type type,
> >> - const u8 *buffer, size_t len)
> >> +static int inno_hdmi_bridge_write_infoframe(struct drm_bridge *bridge,
> >> + enum hdmi_infoframe_type type,
> >> + const u8 *buffer, size_t len)
> >> {
> >> - struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
> >> + struct inno_hdmi *hdmi = bridge_to_inno_hdmi(bridge);
> >> ssize_t i;
> >>
> >> if (type != HDMI_INFOFRAME_TYPE_AVI) {
> >> - drm_err(connector->dev,
> >> - "Unsupported infoframe type: %u\n", type);
> >> + drm_err(bridge->dev, "Unsupported infoframe type: %u\n", type);
> >> return 0;
> >> }
> >>
> >> - inno_hdmi_disable_frame(connector, type);
> >> + inno_hdmi_bridge_clear_infoframe(bridge, type);
> >>
> >> for (i = 0; i < len; i++)
> >> hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i, buffer[i]);
> >
> >It's not an issue for this patch (and I think it can be fixed after this
> >series is merged). I took a quick glance at frame programming. It feels
> >like the clear_infoframe should be poking at registers 0x9c / 0x9d. And
> >write_infoframe then can support HDMI, SPD and Audio infoframes in
> >addition to the AVI. I don't have hardware to experiment (nor time :-)),
> >but would there be a chance to improve this?
>
> Okay, I'll keep your suggestions in mind and look for an opportunity to try them out later.
> The hardware for this board is indeed very scarce at the moment—I put in a lot of effort just
> to get my hands on one. The main reason I'm modifying this code is to convert all of Rockchip's
> display interface drivers into bridge mode, which will make it easier to separate the connector
> part into the display driver side in the future.
No worries, it was just a quick observation from some TRM excerpts that
I found.
--
With best wishes
Dmitry
Powered by blists - more mailing lists