mbox series

[00/12] drm/amd: Switch over to struct drm_edid

Message ID 20240818-amdgpu-drm_edid-v1-0-aea66c1f7cf4@weissschuh.net (mailing list archive)
Headers show
Series drm/amd: Switch over to struct drm_edid | expand

Message

Thomas Weißschuh Aug. 18, 2024, 10:43 a.m. UTC
The AMD DRM drivers use 'struct edid', raw pointers and even custom
structs to represent EDID data.
Uniformly switch to the safe and recommended "struct drm_edid".

Some uses of "struct edid" are left because some ad-hoc parsing is still
being done inside the drivers.

The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
will conflict with my backlight quirk series [0].
The conflict will result in an obvious and easy to fix build failure.

Patches 1 and 2 delete some dead code.
Patches 3 to 6 constify some arguments and shuffle around some code.
The remaining patches perform the actual conversion in steps.

[0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (12):
      drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
      drm/amd/display: Remove EDID members of ddc_service
      drm/edid: constify argument of drm_edid_is_valid()
      drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
      drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
      drm/amd/display: Constify 'struct edid' in parsing functions
      drm/amd/display: Use struct edid in dc_link_add_remote_sink()
      drm/amdgpu: Switch amdgpu_connector to struct drm_edid
      drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
      drm/edid: add a helper to compare two EDIDs
      drm/amd/display: Switch dc_sink to struct drm_edid
      drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid

 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
 .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
 drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
 drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
 drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
 drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
 drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
 .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
 .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
 drivers/gpu/drm/drm_edid.c                         | 20 +++++-
 include/drm/drm_edid.h                             |  3 +-
 20 files changed, 155 insertions(+), 171 deletions(-)
---
base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
change-id: 20240615-amdgpu-drm_edid-32d969dfb899

Best regards,

Comments

Melissa Wen Aug. 19, 2024, 2:31 p.m. UTC | #1
On 08/18, Thomas Weißschuh wrote:
> The AMD DRM drivers use 'struct edid', raw pointers and even custom
> structs to represent EDID data.
> Uniformly switch to the safe and recommended "struct drm_edid".
> 
> Some uses of "struct edid" are left because some ad-hoc parsing is still
> being done inside the drivers.

Hi Thomas,

It's great to see more people working on removing raw edid from amd
display driver in favor of drm_edid.

I glanced over your series and I found it similar to my recent proposal
to migrate amdgpu_dm_connector from edid to drm_edid. You can find the
v5 of this work here:
https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/

I believe it's more productive if we can join efforts and improve that
proposal instead of duplicating work. I'll look at your patches more
carefully this week. If you can review my work, I'd be happy to hear
your feedback too.

Thanks,

Melissa

> 
> The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
> will conflict with my backlight quirk series [0].
> The conflict will result in an obvious and easy to fix build failure.
> 
> Patches 1 and 2 delete some dead code.
> Patches 3 to 6 constify some arguments and shuffle around some code.
> The remaining patches perform the actual conversion in steps.
> 
> [0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Thomas Weißschuh (12):
>       drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
>       drm/amd/display: Remove EDID members of ddc_service
>       drm/edid: constify argument of drm_edid_is_valid()
>       drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
>       drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
>       drm/amd/display: Constify 'struct edid' in parsing functions
>       drm/amd/display: Use struct edid in dc_link_add_remote_sink()
>       drm/amdgpu: Switch amdgpu_connector to struct drm_edid
>       drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
>       drm/edid: add a helper to compare two EDIDs
>       drm/amd/display: Switch dc_sink to struct drm_edid
>       drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
>  .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
>  drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
>  drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
>  drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
>  drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
>  drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
>  .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
>  .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
>  drivers/gpu/drm/drm_edid.c                         | 20 +++++-
>  include/drm/drm_edid.h                             |  3 +-
>  20 files changed, 155 insertions(+), 171 deletions(-)
> ---
> base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
> change-id: 20240615-amdgpu-drm_edid-32d969dfb899
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
>
Thomas Weißschuh Aug. 19, 2024, 6:39 p.m. UTC | #2
Hi Melissa,

On 2024-08-19 11:31:44+0000, Melissa Wen wrote:
> On 08/18, Thomas Weißschuh wrote:
> > The AMD DRM drivers use 'struct edid', raw pointers and even custom
> > structs to represent EDID data.
> > Uniformly switch to the safe and recommended "struct drm_edid".
> > 
> > Some uses of "struct edid" are left because some ad-hoc parsing is still
> > being done inside the drivers.
> 
> Hi Thomas,
> 
> It's great to see more people working on removing raw edid from amd
> display driver in favor of drm_edid.
> 
> I glanced over your series and I found it similar to my recent proposal
> to migrate amdgpu_dm_connector from edid to drm_edid. You can find the
> v5 of this work here:
> https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/

thanks for the pointer.

> I believe it's more productive if we can join efforts and improve that
> proposal instead of duplicating work. I'll look at your patches more
> carefully this week. If you can review my work, I'd be happy to hear
> your feedback too.

Indeed. I'll take a look at your patches.
Let's see how they can be combined.

> Thanks,
> 
> Melissa
> 
> > 
> > The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
> > will conflict with my backlight quirk series [0].
> > The conflict will result in an obvious and easy to fix build failure.
> > 
> > Patches 1 and 2 delete some dead code.
> > Patches 3 to 6 constify some arguments and shuffle around some code.
> > The remaining patches perform the actual conversion in steps.
> > 
> > [0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Thomas Weißschuh (12):
> >       drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
> >       drm/amd/display: Remove EDID members of ddc_service
> >       drm/edid: constify argument of drm_edid_is_valid()
> >       drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
> >       drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
> >       drm/amd/display: Constify 'struct edid' in parsing functions
> >       drm/amd/display: Use struct edid in dc_link_add_remote_sink()
> >       drm/amdgpu: Switch amdgpu_connector to struct drm_edid
> >       drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
> >       drm/edid: add a helper to compare two EDIDs
> >       drm/amd/display: Switch dc_sink to struct drm_edid
> >       drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
> >  .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
> >  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
> >  .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
> >  drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
> >  drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
> >  drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
> >  drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
> >  drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
> >  .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
> >  .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
> >  drivers/gpu/drm/drm_edid.c                         | 20 +++++-
> >  include/drm/drm_edid.h                             |  3 +-
> >  20 files changed, 155 insertions(+), 171 deletions(-)
> > ---
> > base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
> > change-id: 20240615-amdgpu-drm_edid-32d969dfb899
> > 
> > Best regards,
> > -- 
> > Thomas Weißschuh <linux@weissschuh.net>
> >