[Intel-gfx] [RFC 4/8] drm/i915: Group gen 10 display.
Lucas De Marchi
lucas.demarchi at intel.com
Fri Oct 19 16:59:46 UTC 2018
On Fri, Oct 19, 2018 at 01:33:08PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 19, 2018 at 10:30:46AM +0200, Daniel Vetter wrote:
> > 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 :-)
>
> There's also these interesting gen designations in some old docs:
> ctg/elk = gen5.5
> ilk = gen5.75
> I guess we could more or less call all of that as gen5 display.
>
> And of course we have the other oddballs like vlv/chv which are
> sort of mismash of i965, ctg/elk, ibx, and custom stuff. Also
> pnv is mostly gen3 display except for a few bits that were
> snatched from i965.
>
> Not sure we have enough numbers for all that without resotring to
> fractions. And no one could anyway remember what all the different
> numbers mean.
I think we should approximate to the closest one, but prefer using gen_display number
checks for cases in which it's clear from the spec the separation.
For other things we will need more fine grained flags, that can be used across
gens or to differentiate features within a single gen.
Lucas De Marchi
>
> >
> > > 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 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
>
> --
> Ville Syrjälä
> Intel
More information about the Intel-gfx
mailing list