Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@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().

We also have HAS_DDI, which I guess you could call gen8 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. ;)

Yeah, stuffing this into intel_info imo makes sense. As long as it's
booleans they should be fairly cheap.
-Daniel
> 
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx