Re: [PATCH v5 3/3] drm/i915: Add background color hardware readout and state check

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

On Thu, Jan 31, 2019 at 03:55:08PM -0800, Matt Roper wrote:
> We should support readout and verification of crtc background color as
> we do with other pipe state.  Note that our hardware holds less bits of
> precision than the CRTC state allows, so we need to take care to only
> verify the most significant bits of the color after performing readout.
> 
> At boot time the pipe color is already sanitized to full black as
> required by ABI, so the new readout here won't break that requirement.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 469480fe01a4..ecbfc8ce7b54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9740,6 +9740,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum intel_display_power_domain power_domain;
>  	u64 power_domain_mask;
> +	u32 bgcolor;
>  	bool active;
>  
>  	intel_crtc_init_scalers(crtc, pipe_config);
> @@ -9806,6 +9807,15 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		bgcolor = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
> +		pipe_config->base.bgcolor =
> +			drm_argb(10, 0xFFFF,
> +				 bgcolor >> 20 & 0x3FF,
> +				 bgcolor >> 10 & 0x3FF,
> +				 bgcolor       & 0x3FF);
> +	}
> +
>  out:
>  	for_each_power_domain(power_domain, power_domain_mask)
>  		intel_display_power_put_unchecked(dev_priv, power_domain);
> @@ -11422,6 +11432,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				      drm_rect_width(&state->base.dst),
>  				      drm_rect_height(&state->base.dst));
>  	}
> +
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		DRM_DEBUG_KMS("background color: %llx\n",
> +			      pipe_config->base.bgcolor);
>  }
>  
>  static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> @@ -11784,6 +11798,16 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
> +	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
> +		pipe_config_err(adjust, __stringify(name), \
> +			  "(expected 0x%016llx, found 0x%016llx)\n", \
> +			  current_config->name & mask, \
> +			  pipe_config->name & mask); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
>  #define PIPE_CONF_CHECK_I(name) do { \
>  	if (current_config->name != pipe_config->name) { \
>  		pipe_config_err(adjust, __stringify(name), \
> @@ -12035,6 +12059,14 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  
>  	PIPE_CONF_CHECK_I(min_voltage_level);
>  
> +	/*
> +	 * Hardware only holds top 10 bits of each color component; ignore
> +	 * bottom six bits (and all of alpha) when comparing against readout.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		PIPE_CONF_CHECK_LLX_MASKED(base.bgcolor, 0x0000FFC0FFC0FFC0);
> +
> +#undef PIPE_CONF_CHECK_LLX_MASKED
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_BOOL
> -- 
> 2.14.5

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx