Re: [PATCH] drm/amd/display: restore edid reading from a given i2c adapter

[Mario Limonciello <mario.limonciello@xxxxxxx>]

On 2/9/2025 16:50, Melissa Wen wrote:
> When switching to drm_edid, we slightly changed how to get edid by
> removing the possibility of getting them from dc_link when in aux
> transaction mode. As MST doesn't initialize the connector with
> `drm_connector_init_with_ddc()`, restore the original behavior to avoid
> functional changes.
>
> Fixes: 48edb2a4256e ("drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid")
> Signed-off-by: Melissa Wen <mwen@xxxxxxxxxx>
> ---
>
>
> Hi,
>
> So far, there is no reports about an issue related to this but I noticed
> this potential functional change when investigating the previous
> freesync problem. I'm not 100% clear if MST takes this path without
> initializating connector->ddc, but I propose here to restore the
> original behavior to avoid regressions.
>
> Melissa

I think this looks reasonable.

Charlie,

Can this get included in this week's promotion tests?

Thanks,

>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 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 a8421c07b160..0cd22a6686a3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7269,8 +7269,14 @@ 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;
> +	struct i2c_adapter *ddc;
>   
> -	drm_edid = drm_edid_read(connector);
> +	if (dc_link->aux_mode)
> +		ddc = &aconnector->dm_dp_aux.aux.ddc;
> +	else
> +		ddc = &aconnector->i2c->base;
> +
> +	drm_edid = drm_edid_read_ddc(connector, ddc);
>   	drm_edid_connector_update(connector, drm_edid);
>   	if (!drm_edid) {
>   		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
> @@ -7315,14 +7321,21 @@ static int get_modes(struct drm_connector *connector)
>   static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
>   {
>   	struct drm_connector *connector = &aconnector->base;
> +	struct dc_link *dc_link = aconnector->dc_link;
>   	struct dc_sink_init_data init_params = {
>   			.link = aconnector->dc_link,
>   			.sink_signal = SIGNAL_TYPE_VIRTUAL
>   	};
>   	const struct drm_edid *drm_edid;
>   	const struct edid *edid;
> +	struct i2c_adapter *ddc;
>   
> -	drm_edid = drm_edid_read(connector);
> +	if (dc_link->aux_mode)
> +		ddc = &aconnector->dm_dp_aux.aux.ddc;
> +	else
> +		ddc = &aconnector->i2c->base;
> +
> +	drm_edid = drm_edid_read_ddc(connector, ddc);
>   	drm_edid_connector_update(connector, drm_edid);
>   	if (!drm_edid) {
>   		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);