Re: [PATCH 1/3] drm/todo: Create a TODO item for additional HDMI work

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

Hi Jani,

On Thu, Jun 13, 2024 at 10:44:42AM GMT, Jani Nikula wrote:
> On Wed, 12 Jun 2024, Maxime Ripard <mripard@kernel.org> wrote:
> > We recently added some infrastructure to deal with HDMI but we're still
> > lacking a couple of things. Add a TODO entry with the remaining items.
> >
> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  Documentation/gpu/todo.rst | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > index 2ea6ffc9b22b..52fd8672fb6d 100644
> > --- a/Documentation/gpu/todo.rst
> > +++ b/Documentation/gpu/todo.rst
> > @@ -633,10 +633,39 @@ long as that supports DMA. Otherwise importing can still needlessly fail.
> >  
> >  Contact: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter
> >  
> >  Level: Advanced
> >  
> > +Improve HDMI Infrastructure
> > +---------------------------
> > +
> > +We have a bunch of helpers to handle HDMI and reduce the boilerplate in
> > +drivers. Support so far includes HDMI 1.4 support, but we need to extend
> > +it with:
> > +
> > +  - CEC handling support. CEC requires a bit of integration into every
> > +    HDMI driver to set the device physical address according to the EDID
> > +    in `.get_modes`, and to clear/reset it in the hotplug detection
> > +    path. We should create the ``drm_atomic_helper_connector_hdmi_get_modes()``
> > +    and ``drm_atomic_helper_connector_hdmi_handle_hotplug()`` helpers to handle
> > +    this properly, and convert drivers to use them.
> 
> Furthermore, we should stop passing EDID to the CEC functions, and
> instead use the source physical address we've parsed ourselves and
> stored to connector->display_info.source_physical_address.
> 
> I.e. stop using
> 
> - drm_dp_cec_set_edid()
> - cec_s_phys_addr_from_edid()
> - cec_get_edid_phys_addr()
> 
> And instead use .source_physical_address and
> 
> - drm_dp_cec_attach()
> - cec_s_phys_addr()
> 
> The main rationale is to avoid using a separate EDID parser that's
> outside of the drm subsystem and unaware of struct drm_edid and frankly
> cdoes not look very robust.

It's a good idea, but imo do not belong in that entry. I've added it to
the last patch of this series which fits better to me.

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]