Re: [PATCH v3 07/11] drm: add helper to get crtc timings

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 24, 2014 at 02:20:28PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> We need to get hdisplay and vdisplay in a few places so create a
> helper to make our job easier.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/drm_crtc.c           | 20 +++++++++++++-------
>  drivers/gpu/drm/i915/intel_display.c |  6 +++---
>  include/drm/drm_crtc.h               |  2 ++
>  3 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b702106..7c0bf9f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2490,6 +2490,17 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
>  }
>  EXPORT_SYMBOL(drm_mode_set_config_internal);
>  
> +void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
> +			    int *hdisplay, int *vdisplay)
> +{
> +	struct drm_display_mode adjusted = *mode;
> +
> +	drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE);
> +	*hdisplay = adjusted.crtc_hdisplay;
> +	*vdisplay = adjusted.crtc_vdisplay;

Hmm. Now that I read drm_mode_set_crtcinfo() it would seem we'll do
the wrong thing when dealing with doublescan or vscan>1. That problem
is already true without your patch, but we should do something to fix
it anyway.

Maybe the best option would be to yank the stereo doubling logic from
drm_mode_set_crtcinfo() into a separate function and call it both here
and in drm_mode_set_crtcinfo(). That would avoid this problem without
duplicating the specifics of stereo doubling in multiple places.

> +}
> +EXPORT_SYMBOL(drm_crtc_get_hv_timing);
> +
>  /**
>   * drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
>   *     CRTC viewport
> @@ -2510,13 +2521,8 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>  	hdisplay = mode->hdisplay;
>  	vdisplay = mode->vdisplay;

With the suggested doublescan/vscan fix you could kill these
assignments too and just call drm_crtc_get_hv_timing() unconditionally.

>  
> -	if (drm_mode_is_stereo(mode)) {
> -		struct drm_display_mode adjusted = *mode;
> -
> -		drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE);
> -		hdisplay = adjusted.crtc_hdisplay;
> -		vdisplay = adjusted.crtc_vdisplay;
> -	}
> +	if (drm_mode_is_stereo(mode))
> +		drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
>  
>  	if (crtc->invert_dimensions)
>  		swap(hdisplay, vdisplay);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3cb092c..dfe9ad5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10154,9 +10154,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	 * computation to clearly distinguish it from the adjusted mode, which
>  	 * can be changed by the connectors in the below retry loop.
>  	 */
> -	drm_mode_set_crtcinfo(&pipe_config->requested_mode, CRTC_STEREO_DOUBLE);
> -	pipe_config->pipe_src_w = pipe_config->requested_mode.crtc_hdisplay;
> -	pipe_config->pipe_src_h = pipe_config->requested_mode.crtc_vdisplay;
> +	drm_crtc_get_hv_timing(&pipe_config->requested_mode,
> +			       &pipe_config->pipe_src_w,
> +			       &pipe_config->pipe_src_h);
>  
>  encoder_retry:
>  	/* Ensure the port clock defaults are reset when retrying. */
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c40070a..9b2f6b5 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -950,6 +950,8 @@ extern int drm_plane_init(struct drm_device *dev,
>  extern void drm_plane_cleanup(struct drm_plane *plane);
>  extern unsigned int drm_plane_index(struct drm_plane *plane);
>  extern void drm_plane_force_disable(struct drm_plane *plane);
> +extern void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
> +				   int *hdisplay, int *vdisplay);
>  extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>  				   int x, int y,
>  				   const struct drm_display_mode *mode,
> -- 
> 1.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC