Re: [RFC PATCH 2/2] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid

2024-02-05 Thread Melissa Wen
On 01/26, Alex Hung wrote:
> 
> 
> On 2024-01-26 09:28, Melissa Wen wrote:
> > Replace raw edid handling (struct edid) with the opaque EDID type
> > (struct drm_edid) on amdgpu_dm_connector for consistency. It may also
> > prevent mismatch of approaches in different parts of the driver code.
> > Working in progress. There are a couple of cast warnings and it was only
> > tested with IGT.
> > 
> > Signed-off-by: Melissa Wen 
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 63 ++-
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  4 +-
> >   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  9 +--
> >   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 23 +++
> >   4 files changed, 51 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 68e71e2ea472..741081d73bb3 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -3277,12 +3277,12 @@ void amdgpu_dm_update_connector_after_detect(
> > &aconnector->dm_dp_aux.aux);
> > }
> > } else {
> > -   aconnector->edid =
> > -   (struct edid *)sink->dc_edid.raw_edid;
> > +   const struct edid *edid = (const struct edid 
> > *)sink->dc_edid.raw_edid;
> > +   aconnector->edid = drm_edid_alloc(edid, 
> > (edid->extensions + 1) * EDID_LENGTH);
> > if (aconnector->dc_link->aux_mode)
> > drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
> > -   aconnector->edid);
> > +   
> > drm_edid_raw(aconnector->edid));
> > }
> > if (!aconnector->timing_requested) {
> > @@ -3293,13 +3293,13 @@ void amdgpu_dm_update_connector_after_detect(
> > "failed to create 
> > aconnector->requested_timing\n");
> > }
> > -   drm_connector_update_edid_property(connector, aconnector->edid);
> > +   drm_edid_connector_update(connector, aconnector->edid);
> > amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
> > update_connector_ext_caps(aconnector);
> > } else {
> > drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
> > amdgpu_dm_update_freesync_caps(connector, NULL);
> > -   drm_connector_update_edid_property(connector, NULL);
> > +   drm_edid_connector_update(connector, NULL);
> > aconnector->num_modes = 0;
> > dc_sink_release(aconnector->dc_sink);
> > aconnector->dc_sink = NULL;
> > @@ -6564,7 +6564,6 @@ static void amdgpu_dm_connector_funcs_force(struct 
> > drm_connector *connector)
> > struct dc_link *dc_link = aconnector->dc_link;
> > struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
> > const struct drm_edid *drm_edid;
> > -   const struct edid *edid;
> > /*
> >  * Note: drm_get_edid gets edid in the following order:
> > @@ -6578,11 +6577,12 @@ static void amdgpu_dm_connector_funcs_force(struct 
> > drm_connector *connector)
> > DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
> > return;
> > }
> > -   edid = drm_edid_raw(drm_edid);
> > -   aconnector->edid = edid;
> > -
> > +   aconnector->edid = drm_edid;
> > +   drm_edid_connector_update(connector, drm_edid);
> > /* Update emulated (virtual) sink's EDID */
> > if (dc_em_sink && dc_link) {
> > +   const struct edid *edid = drm_edid_raw(drm_edid);
> > +
> > memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
> > memmove(dc_em_sink->dc_edid.raw_edid, (uint8_t *)edid, 
> > (edid->extensions + 1) * EDID_LENGTH);
> > dm_helpers_parse_edid_caps(
> > @@ -6633,13 +6633,13 @@ static void create_eml_sink(struct 
> > amdgpu_dm_connector *aconnector)
> > return;
> > }
> > -   edid = drm_edid_raw(drm_edid);
> > -
> > -   if (drm_detect_hdmi_monitor(edid))
> > +   if (connector->display_info.is_hdmi)
> > init_params.sink_signal = SIGNAL_TYPE_HDMI_TYPE_A;
> > -   aconnector->edid = edid;
> > +   aconnector->edid = drm_edid;
> > +   drm_edid_connector_update(connector, drm_edid);
> > +   edid = drm_edid_raw(drm_edid);
> > aconnector->dc_em_sink = dc_link_add_remote_sink(
> > aconnector->dc_link,
> > (uint8_t *)edid,
> > @@ -7322,16 +7322,16 @@ static void amdgpu_set_panel_orientation(struct 
> > drm_connector *connector)
> >   }
> >   static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector 
> > *connector,
> > - struct edid *edid)
> > + const struct drm_edid *drm_edid)
> >   {
> > struct a

Re: [RFC PATCH 2/2] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid

2024-01-26 Thread Alex Hung




On 2024-01-26 09:28, Melissa Wen wrote:

Replace raw edid handling (struct edid) with the opaque EDID type
(struct drm_edid) on amdgpu_dm_connector for consistency. It may also
prevent mismatch of approaches in different parts of the driver code.
Working in progress. There are a couple of cast warnings and it was only
tested with IGT.

Signed-off-by: Melissa Wen 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 63 ++-
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  4 +-
  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  9 +--
  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 23 +++
  4 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 68e71e2ea472..741081d73bb3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3277,12 +3277,12 @@ void amdgpu_dm_update_connector_after_detect(
&aconnector->dm_dp_aux.aux);
}
} else {
-   aconnector->edid =
-   (struct edid *)sink->dc_edid.raw_edid;
+   const struct edid *edid = (const struct edid 
*)sink->dc_edid.raw_edid;
+   aconnector->edid = drm_edid_alloc(edid, 
(edid->extensions + 1) * EDID_LENGTH);
  
  			if (aconnector->dc_link->aux_mode)

drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
-   aconnector->edid);
+   
drm_edid_raw(aconnector->edid));
}
  
  		if (!aconnector->timing_requested) {

@@ -3293,13 +3293,13 @@ void amdgpu_dm_update_connector_after_detect(
"failed to create 
aconnector->requested_timing\n");
}
  
-		drm_connector_update_edid_property(connector, aconnector->edid);

+   drm_edid_connector_update(connector, aconnector->edid);
amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
update_connector_ext_caps(aconnector);
} else {
drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
amdgpu_dm_update_freesync_caps(connector, NULL);
-   drm_connector_update_edid_property(connector, NULL);
+   drm_edid_connector_update(connector, NULL);
aconnector->num_modes = 0;
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
@@ -6564,7 +6564,6 @@ static void amdgpu_dm_connector_funcs_force(struct 
drm_connector *connector)
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
const struct drm_edid *drm_edid;
-   const struct edid *edid;
  
  	/*

 * Note: drm_get_edid gets edid in the following order:
@@ -6578,11 +6577,12 @@ static void amdgpu_dm_connector_funcs_force(struct 
drm_connector *connector)
DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
}
-   edid = drm_edid_raw(drm_edid);
-   aconnector->edid = edid;
-
+   aconnector->edid = drm_edid;
+   drm_edid_connector_update(connector, drm_edid);
/* Update emulated (virtual) sink's EDID */
if (dc_em_sink && dc_link) {
+   const struct edid *edid = drm_edid_raw(drm_edid);
+
memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
memmove(dc_em_sink->dc_edid.raw_edid, (uint8_t *)edid, 
(edid->extensions + 1) * EDID_LENGTH);
dm_helpers_parse_edid_caps(
@@ -6633,13 +6633,13 @@ static void create_eml_sink(struct amdgpu_dm_connector 
*aconnector)
return;
}
  
-	edid = drm_edid_raw(drm_edid);

-
-   if (drm_detect_hdmi_monitor(edid))
+   if (connector->display_info.is_hdmi)
init_params.sink_signal = SIGNAL_TYPE_HDMI_TYPE_A;
  
-	aconnector->edid = edid;

+   aconnector->edid = drm_edid;
+   drm_edid_connector_update(connector, drm_edid);
  
+	edid = drm_edid_raw(drm_edid);

aconnector->dc_em_sink = dc_link_add_remote_sink(
aconnector->dc_link,
(uint8_t *)edid,
@@ -7322,16 +7322,16 @@ static void amdgpu_set_panel_orientation(struct 
drm_connector *connector)
  }
  
  static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,

- struct edid *edid)
+ const struct drm_edid *drm_edid)
  {
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
  
-	if (edid) {

+   if (drm_edid) {
/* empty probed_modes */
INIT_LIST_HEAD(&connector->pro