Re: [Intel-gfx] [PATCH 10/22] drm/msm: Nuke weird on stack mode copy

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 18/02/2022 13:03, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This on stack middle man mode looks entirely pointless.
> Just duplicate the original mode directly.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I took a glance at the surrounding piece of code.
The dp_connector_get_modes() calls dp_display_get_modes() in attempt to 
fill the dp_mode argument. However the dp_display_get_modes() function 
just calls dp_panel_get_modes(), which does not touch dp_mode argument 
since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in 
case of DP CTS") dating September 2020. I think we can drop this piece 
of code completely.

> ---
>   drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> index d4d360d19eba..09188d02aa1e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -56,7 +56,7 @@ static int dp_connector_get_modes(struct drm_connector *connector)
>   	int rc = 0;
>   	struct msm_dp *dp;
>   	struct dp_display_mode *dp_mode = NULL;
> -	struct drm_display_mode *m, drm_mode;
> +	struct drm_display_mode *m;
>   
>   	if (!connector)
>   		return 0;
> @@ -82,13 +82,11 @@ static int dp_connector_get_modes(struct drm_connector *connector)
>   			return rc;
>   		}
>   		if (dp_mode->drm_mode.clock) { /* valid DP mode */
> -			memset(&drm_mode, 0x0, sizeof(drm_mode));
> -			drm_mode_copy(&drm_mode, &dp_mode->drm_mode);
> -			m = drm_mode_duplicate(connector->dev, &drm_mode);
> +			m = drm_mode_duplicate(connector->dev, &dp_mode->drm_mode);
>   			if (!m) {
>   				DRM_ERROR("failed to add mode %ux%u\n",
> -				       drm_mode.hdisplay,
> -				       drm_mode.vdisplay);
> +					  dp_mode->drm_mode.hdisplay,
> +					  dp_mode->drm_mode.vdisplay);
>   				kfree(dp_mode);
>   				return 0;
>   			}


-- 
With best wishes
Dmitry