Re: [PATCH 1/3] drm/i915/display: Modify panel_fitting code for joiner

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

On Wed, Sep 25, 2024 at 12:00:30PM +0530, Nemesa Garg wrote:
> Replace adjusted_mode with pipe_mode in pch_panel_fitting
> so as to that final pipe src width and height is used after
> joiner calculation. De-couple the current intel_panel_fitting
> function, one pre-ilk and one post-ilk, as post-ilk
> pch_panel_fitting is called from pipe_config.
> 
> v4: Replace adjusted_mode with pipe_mode[Ville]
>     Keep gmch panel fitting in same place[Ville]
> 
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 52 +++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_panel.h |  8 +++-
>  2 files changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 71454ddef20f..bd25c96f2e57 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -387,15 +387,15 @@ void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
>  static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  			     const struct drm_connector_state *conn_state)
>  {
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> +	const struct drm_display_mode *pipe_mode =
> +		&crtc_state->hw.pipe_mode;

We don't have that where this currently gets caller.

We'll need to do this in careful steps:
1) reject joiner + pfit (assuming we are allowing this currently?)
   needs to be first so we can backport it
2) call pch_panel_fitting() after pipe_src+pipe_mode have been
   computed
3) switch to using pipe_mode in pch_panel_fitting()
4) allow joiner+pfit again, assuming everyhting looks kosher

5) ponteially follow up with some cleanups, eg. get rid of the
   early pipe_src initialization in intel_modeset_pipe_config().
   This needs to be done without breaking the gmch stuff mind you,
   as that would presumably still need pipe_src early

>  	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
>  	int pipe_src_h = drm_rect_height(&crtc_state->pipe_src);
>  	int x, y, width, height;
>  
>  	/* Native modes don't need fitting */
> -	if (adjusted_mode->crtc_hdisplay == pipe_src_w &&
> -	    adjusted_mode->crtc_vdisplay == pipe_src_h &&
> +	if (pipe_mode->crtc_hdisplay == pipe_src_w &&
> +	    pipe_mode->crtc_vdisplay == pipe_src_h &&
>  	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
>  		return 0;
>  
> @@ -403,45 +403,45 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  	case DRM_MODE_SCALE_CENTER:
>  		width = pipe_src_w;
>  		height = pipe_src_h;
> -		x = (adjusted_mode->crtc_hdisplay - width + 1)/2;
> -		y = (adjusted_mode->crtc_vdisplay - height + 1)/2;
> +		x = (pipe_mode->crtc_hdisplay - width + 1) / 2;
> +		y = (pipe_mode->crtc_vdisplay - height + 1) / 2;
>  		break;
>  
>  	case DRM_MODE_SCALE_ASPECT:
>  		/* Scale but preserve the aspect ratio */
>  		{
> -			u32 scaled_width = adjusted_mode->crtc_hdisplay * pipe_src_h;
> -			u32 scaled_height = pipe_src_w * adjusted_mode->crtc_vdisplay;
> +			u32 scaled_width = pipe_mode->crtc_hdisplay * pipe_src_h;
> +			u32 scaled_height = pipe_src_w * pipe_mode->crtc_vdisplay;
>  			if (scaled_width > scaled_height) { /* pillar */
>  				width = scaled_height / pipe_src_h;
>  				if (width & 1)
>  					width++;
> -				x = (adjusted_mode->crtc_hdisplay - width + 1) / 2;
> +				x = (pipe_mode->crtc_hdisplay - width + 1) / 2;
>  				y = 0;
> -				height = adjusted_mode->crtc_vdisplay;
> +				height = pipe_mode->crtc_vdisplay;
>  			} else if (scaled_width < scaled_height) { /* letter */
>  				height = scaled_width / pipe_src_w;
>  				if (height & 1)
>  				    height++;
> -				y = (adjusted_mode->crtc_vdisplay - height + 1) / 2;
> +				y = (pipe_mode->crtc_vdisplay - height + 1) / 2;
>  				x = 0;
> -				width = adjusted_mode->crtc_hdisplay;
> +				width = pipe_mode->crtc_hdisplay;
>  			} else {
>  				x = y = 0;
> -				width = adjusted_mode->crtc_hdisplay;
> -				height = adjusted_mode->crtc_vdisplay;
> +				width = pipe_mode->crtc_hdisplay;
> +				height = pipe_mode->crtc_vdisplay;
>  			}
>  		}
>  		break;
>  
>  	case DRM_MODE_SCALE_NONE:
> -		WARN_ON(adjusted_mode->crtc_hdisplay != pipe_src_w);
> -		WARN_ON(adjusted_mode->crtc_vdisplay != pipe_src_h);
> +		WARN_ON(pipe_mode->crtc_hdisplay != pipe_src_w);
> +		WARN_ON(pipe_mode->crtc_vdisplay != pipe_src_h);
>  		fallthrough;
>  	case DRM_MODE_SCALE_FULLSCREEN:
>  		x = y = 0;
> -		width = adjusted_mode->crtc_hdisplay;
> -		height = adjusted_mode->crtc_vdisplay;
> +		width = pipe_mode->crtc_hdisplay;
> +		height = pipe_mode->crtc_vdisplay;
>  		break;
>  
>  	default:
> @@ -666,16 +666,16 @@ static int gmch_panel_fitting(struct intel_crtc_state *crtc_state,
>  	return 0;
>  }
>  
> -int intel_panel_fitting(struct intel_crtc_state *crtc_state,
> -			const struct drm_connector_state *conn_state)
> +int intel_gch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	return gmch_panel_fitting(crtc_state, conn_state);
> +}
>  
> -	if (HAS_GMCH(i915))
> -		return gmch_panel_fitting(crtc_state, conn_state);
> -	else
> -		return pch_panel_fitting(crtc_state, conn_state);
> +int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state)
> +{
> +	return pch_panel_fitting(crtc_state, conn_state);
>  }
>  
>  enum drm_connector_status
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
> index 15a8c897b33f..0f678cd72403 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -42,8 +42,12 @@ enum drrs_type intel_panel_drrs_type(struct intel_connector *connector);
>  enum drm_mode_status
>  intel_panel_mode_valid(struct intel_connector *connector,
>  		       const struct drm_display_mode *mode);
> -int intel_panel_fitting(struct intel_crtc_state *crtc_state,
> -			const struct drm_connector_state *conn_state);
> +int intel_gch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state);
> +
> +int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state);
> +
>  int intel_panel_compute_config(struct intel_connector *connector,
>  			       struct drm_display_mode *adjusted_mode);
>  void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel