[Intel-gfx] [RFC 4/8] drm/i915: Group gen 10 display.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> Continuing with the goal of use less platform codenames:
> let's group platforms who has gen10 display.

Ahah, so this answers my question in the previous patch. ;)

So we already have HAS_GMCH_DISPLAY().

If we're going to make gen10 display a thing, should we not generalize
this to have an actual display gen in device info? Of course for most
platforms it's going to be identical to the gem gen.

The question becomes, is display gen accurate enough? It's not enough to
cover all of Geminilake I believe. Which makes me think, should we just
add tons more device info flags to cover features at a detailed level? I
think that's what Joonas advocates. Perhaps it bloats the device info,
and causes increase in the number of device info blocks. But my gut
feeling says together with truly immutable device info, there are
compiler optimization benefits to be had.

Also, currently we "inherit" device info using truly obnoxious macro
layering where you have to work hard to trace the macros to find out
what is set for a given platform. It doesn't have to be that way. We
could move parts of it to separate but shared features structs, and add
pointers to them.

Anyway, thanks for rolling this series as a starting point for
discussion. Even if I'm not buying the changes as-is. ;)


BR,
Jani.

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
>  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
>  drivers/gpu/drm/i915/intel_color.c   | 2 +-
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
>  5 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d19b38ef145b..6436fedfe561 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
>  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
>  
> +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> +
>  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
>  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
>  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 4aa6500604cc..b315b70fd49c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	    crtc_state->has_audio &&
>  	    crtc_state->port_clock >= 540000 &&
>  	    crtc_state->lane_count == 4) {
> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +		if (HAS_DISPLAY_10(dev_priv)) {
>  			/* Display WA #1145: glk,cnl */
>  			min_cdclk = max(316800, min_cdclk);
>  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 5127da286a2b..d5f67b9c9698 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
>  		   IS_BROXTON(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = broadwell_load_luts;
> -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> +	} else if (HAS_DISPLAY_10(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = glk_load_luts;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7b04b8436b6d..1abf79a4ee91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_crtc->active = true;
>  
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
>  			 pipe_config->pch_pfit.enabled;
>  	if (psl_clkgate_wa)
>  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
>  static void intel_early_display_was(struct drm_i915_private *dev_priv)
>  {
>  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +	if (HAS_DISPLAY_10(dev_priv))
>  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
>  			   DARBF_GATING_DIS);
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 7cd59eee5cad..912ab7b9d89a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
>  	 * than the cursor ending less than 4 pixels from the left edge of the
>  	 * screen may cause FIFO underflow and display corruption.
>  	 */
> -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +	if (HAS_DISPLAY_10(dev_priv) &&
>  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
>  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
>  			      crtc_x + crtc_w < 4 ? "end" : "start",

-- 
Jani Nikula, Intel Open Source Graphics Center