Re: [Intel-gfx] [PATCH 3/6] drm/i915/display: global histogram restrictions

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 18 May 2023, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> For global histogram the panel should be edp and should have pwm based
> backlight controller. Flags are updated accordingly.
>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index cd21b0ddbabb..975d6bdb59f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -17,12 +17,14 @@
>  #include "intel_crtc_state_dump.h"
>  #include "intel_ddi.h"
>  #include "intel_de.h"
> +#include "intel_dp.h"
>  #include "intel_display.h"
>  #include "intel_display_power.h"
>  #include "intel_display_types.h"
>  #include "intel_modeset_setup.h"
>  #include "intel_pch_display.h"
>  #include "intel_pm.h"
> +#include "intel_global_hist.h"
>  
>  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  					struct drm_modeset_acquire_ctx *ctx)
> @@ -309,6 +311,7 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  	struct intel_crtc_state *crtc_state = crtc ?
>  		to_intel_crtc_state(crtc->base.state) : NULL;
> +	struct intel_panel *panel;
>  
>  	/*
>  	 * We need to check both for a crtc link (meaning that the encoder is
> @@ -376,6 +379,15 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
>  
>  	if (HAS_DDI(i915))
>  		intel_ddi_sanitize_encoder_pll_mapping(encoder);
> +
> +	/* validate the global hist struct elements */
> +	if (intel_dp_is_port_edp(i915, encoder->port)) {
> +		crtc->global_hist->has_edp = true;
> +		panel = &connector->panel;
> +		if (panel->backlight.present == true)
> +			crtc->global_hist->has_pwm = true;
> +	}

Again, the dependency on eDP and backlight is unnecessary policy.

Side note, backlight being present doesn't mean "has pwm". It could be
some other kind of backlight as well.

BR,
Jani.

> +
>  }
>  
>  /* FIXME read out full plane state for all planes */

-- 
Jani Nikula, Intel Open Source Graphics Center