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@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 :-)
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.
>
> > 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@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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
