Re: [PATCH 2/5] drm/amdgpu: don't change mode in amdgpu_dm_connector_mode_valid()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Dmitry,

Thank you for the patch.

On Fri, Nov 15, 2024 at 11:09:27PM +0200, Dmitry Baryshkov wrote:
> Make amdgpu_dm_connector_mode_valid() duplicate the mode during the
> test rather than modifying the passed mode. This is a preparation to
> converting the mode_valid() callback of drm_connector to accept const
> struct drm_display_mode argument.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++++++++--
>  1 file changed, 8 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 75d6b90104f8fe196df06383b20ee88196a700bf..d0ca905e91eafe6c53f3f2ebdf3f2ae9589d7f89 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7381,6 +7381,7 @@ enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector *connec
>  {
>  	int result = MODE_ERROR;
>  	struct dc_sink *dc_sink;
> +	struct drm_display_mode *test_mode;
>  	/* TODO: Unhardcode stream count */
>  	struct dc_stream_state *stream;
>  	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
> @@ -7405,11 +7406,16 @@ enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector *connec
>  		goto fail;
>  	}
>  
> -	drm_mode_set_crtcinfo(mode, 0);
> +	test_mode = drm_mode_duplicate(connector->dev, mode);
> +	if (!test_mode)
> +		goto fail;
> +
> +	drm_mode_set_crtcinfo(test_mode, 0);

I wonder if things could be refactored further to avoid the need to
duplicate the mode here, but that seems out of scope for this patch
series.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>  
> -	stream = create_validate_stream_for_sink(aconnector, mode,
> +	stream = create_validate_stream_for_sink(aconnector, test_mode,
>  						 to_dm_connector_state(connector->state),
>  						 NULL);
> +	drm_mode_destroy(connector->dev, test_mode);
>  	if (stream) {
>  		dc_stream_release(stream);
>  		result = MODE_OK;
> 

-- 
Regards,

Laurent Pinchart