linux-kernel - Re: [PATCH v5 9/9] drm/vc4: hdmi: use drm_atomic_helper_connector_hdmi_update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20241206-speedy-beaver-from-asgard-dbed9c@houat>
Date: Fri, 6 Dec 2024 15:23:01 +0100
From: mripard@...nel.org
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, 
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Phong LE <ple@...libre.com>, 
	Inki Dae <inki.dae@...sung.com>, Seung-Woo Kim <sw0312.kim@...sung.com>, 
	Kyungmin Park <kyungmin.park@...sung.com>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Alim Akhtar <alim.akhtar@...sung.com>, Russell King <linux@...linux.org.uk>, 
	Chun-Kuang Hu <chunkuang.hu@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Sandy Huang <hjc@...k-chips.com>, 
	Heiko Stübner <heiko@...ech.de>, Andy Yan <andy.yan@...k-chips.com>, 
	Alain Volmat <alain.volmat@...s.st.com>, Raphael Gallais-Pou <rgallaispou@...il.com>, 
	Dave Stevenson <dave.stevenson@...pberrypi.com>, Maíra Canal <mcanal@...lia.com>, 
	Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, Jani Nikula <jani.nikula@...ux.intel.com>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, 
	linux-mediatek@...ts.infradead.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v5 9/9] drm/vc4: hdmi: use
 drm_atomic_helper_connector_hdmi_update_edid()

On Tue, Dec 03, 2024 at 02:27:44PM +0200, Dmitry Baryshkov wrote:
> On Mon, Dec 02, 2024 at 02:27:49PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > On Sun, Dec 01, 2024 at 02:44:13AM +0200, Dmitry Baryshkov wrote:
> > > Use the helper function to update the connector's information. This
> > > makes sure that HDMI-related events are handled in a generic way.
> > > Currently it is limited to the HDMI state reporting to the sound system.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > > ---
> > >  drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index d0a9aff7ad43016647493263c00d593296a1e3ad..d83f587ab69f4b8f7d5c37a00777f11da8301bc1 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -401,13 +401,16 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> > >  	 */
> > >  
> > >  	if (status == connector_status_disconnected) {
> > > +		drm_atomic_helper_connector_hdmi_update_edid(connector, NULL);
> > >  		cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> > >  		return;
> > >  	}
> > >  
> > >  	drm_edid = drm_edid_read_ddc(connector, vc4_hdmi->ddc);
> > >  
> > > -	drm_edid_connector_update(connector, drm_edid);
> > > +	// TODO: use drm_atomic_helper_connector_hdmi_update() once it gains
> > > +	// CEC support
> > > +	drm_atomic_helper_connector_hdmi_update_edid(connector, drm_edid);
> > 
> > So, it's not just about EDID, and I think we shouldn't really focus on
> > that either.
> > 
> > As that patch points out, even if we only consider EDID's, we have
> > different path depending on the connector status. It shouldn't be up to
> > the drivers to get this right.
> > 
> > What I had in mind was something like a
> > drm_atomic_helper_connector_hdmi_hotplug function that would obviously
> > deal with EDID only here, but would expand to CEC, scrambling, etc.
> > later on.
> 
> I thought about it, after our discussion, but in the end I had to
> implement the EDID-specific function, using edid == NULL as
> "disconnected" event. The issue is pretty simple: there is no standard
> way to get EDID from the connector. The devices can call
> drm_edid_read(), drm_edid_read_ddc(connector->ddc) or (especially
> embedded bridges) use drm_edid_read_custom().

And that's fine, it's to be expected.

> Of course we can go with the functional way and add the
> .read_edid(drm_connector) callback to the HDMI funcs. Then the
> drm_atomic_helper_connector_hdmi_hotplug() function can read EDID on its
> own.

Yep, that's definitely what we should do. And then we can make a
get_modes helper too that would also use it.

> Also the function that you proposed perfectly fits the HPD notification
> callbacks, but which function should be called from the .get_modes()?
> The _hdmi_hotplug() doesn't fit there. Do we still end up with both
> drm_atomic_helper_connector_hdmi_hotplug() and
> drm_atomic_helper_connector_hdmi_update_edid()?

I'd say both a get_modes helper and a hotplug helper, both using that
read_edid hook under the hood.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)