On 2023-09-05 13:13, Alex Hung wrote:
> [WHY]
> edid_override and drm_edid_override_connector_update, according to drm
> documentation, should not be referred outside drm_edid.
> 
> [HOW]
> Remove and replace them accordingly.
> 
> Signed-off-by: Alex Hung <alex.h...@amd.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-----------------
>  1 file changed, 2 insertions(+), 21 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 1bb1a394f55f..f6a255773242 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6372,15 +6372,12 @@ amdgpu_dm_connector_late_register(struct 
> drm_connector *connector)
>  static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
>  {
>       struct amdgpu_dm_connector *aconnector = 
> to_amdgpu_dm_connector(connector);
> +     struct amdgpu_connector *amdgpu_connector = 
> to_amdgpu_connector(connector);
>       struct dc_link *dc_link = aconnector->dc_link;
>       struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
>       struct edid *edid;
>  
> -     if (!connector->edid_override)
> -             return;
> -
> -     drm_edid_override_connector_update(&aconnector->base);
> -     edid = aconnector->base.edid_blob_ptr->data;
> +     edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);

Looks like we only call this in the case where a connector is forced, so
drm_get_edid will never try to read the edid from the ddc but always gives
us the override_edid. Please spell that out in the commit description so
anyone else looking at the patch doesn't have to trace it themselves.

>       aconnector->edid = edid;
>  
>       /* Update emulated (virtual) sink's EDID */
> @@ -6421,22 +6418,6 @@ static void create_eml_sink(struct amdgpu_dm_connector 
> *aconnector)
>       };
>       struct edid *edid;
>  
> -     if (!aconnector->base.edid_blob_ptr) {

Can edid_blob_ptr never be NULL?

Harry

> -             /* if connector->edid_override valid, pass
> -              * it to edid_override to edid_blob_ptr
> -              */
> -
> -             drm_edid_override_connector_update(&aconnector->base);
> -
> -             if (!aconnector->base.edid_blob_ptr) {
> -                     DRM_ERROR("No EDID firmware found on connector: %s 
> ,forcing to OFF!\n",
> -                                     aconnector->base.name);
> -
> -                     aconnector->base.force = DRM_FORCE_OFF;
> -                     return;
> -             }
> -     }
> -
>       edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
>  
>       aconnector->edid = edid;

Reply via email to