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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Like Jani mentioned. I'm all in favour of going with a number of has_*
flags, when there is mix and match between different gens.

Regards, Joonas

> -Daniel
> > 
> > 
> > BR,
> > Jani.
> > 
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > ---
> > >  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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux