[Intel-gfx] [RFC 4/8] drm/i915: Group gen 10 display.
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Fri Oct 19 09:32:01 UTC 2018
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 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().
>
> 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 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
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list