| 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 |
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> >
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> > >
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,