Re: [PATCH v4 3/3] drm/i915/display: Call panel_fitting from pipe_config

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

On Wed, Sep 25, 2024 at 12:00:32PM +0530, Nemesa Garg wrote:
> In panel fitter/pipe scaler scenario the pch_pfit configuration
> currently takes place before accounting for pipe_src width for
> joiner. This causes issue when pch_pfit and joiner get enabled
> together. Call panel_fitting from pipe_config once pipe src is
> computed.
> 
> v4: Remove need_joiner flag [Ville]
> 
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b4ef4d59da1a..0148939caaaa 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4653,6 +4653,8 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct drm_connector *connector;
>  	struct drm_connector_state *connector_state;
> +	const struct drm_display_mode *fixed_mode;
> +	struct intel_dp *intel_dp;
>  	int pipe_src_w, pipe_src_h;
>  	int base_bpp, ret, i;
>  
> @@ -4774,6 +4776,24 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
>  		return ret;
>  	}
>  
> +	for_each_new_connector_in_state(&state->base, connector,
> +					connector_state, i) {
> +		struct intel_encoder *encoder =
> +			to_intel_encoder(connector_state->best_encoder);
> +
> +		if (connector_state->crtc != &crtc->base)
> +			continue;
> +
> +		intel_dp = enc_to_intel_dp(encoder);
> +
> +		if (!intel_dp)
> +			continue;
> +
> +		if ((intel_dp_is_edp(intel_dp) && fixed_mode) ||
> +		    crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)

We don't need those checks, but we do need a !gmch check.

> +			return intel_pch_panel_fitting(crtc_state, connector_state);

That unconditional return should not be here as we'll
now skip the rest of the function.

The connector_state requirement is a bit unfortunate. Not sure
if we should hide this all in the pfit code or not. Hmm, and 
when we introduce the TV margin properties those too will have
to come from the connector state. I guess keeping the loop
in the caller is easier for now at least.

But this is the wrong place to do this all. It needs to be
done between intel_crtc_compute_pipe_mode() and
intel_crtc_compute_pixel_rate(). I would stash it all inside
some kind of intel_crtc_compute_pfit() function so as to
not pollute intel_crtc_compute_config() too badly.

> +	}
> +
>  	/* Dithering seems to not pass-through bits correctly when it should, so
>  	 * only enable it on 6bpc panels and when its not a compliance
>  	 * test requesting 6bpc video pattern.
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel