[Intel-gfx] [RFC 4/8] drm/i915: Group gen 10 display.
Lucas De Marchi
lucas.demarchi at intel.com
Fri Oct 19 16:52:37 UTC 2018
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().
>
> 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.
I think we should have indeed a display_gen in device info. It's a way to group features
that are particular to that gen. Just like we do for gem. When the spec is clear about
this being something from that gen, I think it's fine.
We will need more fine grained flags in the device info, but IMO that should be in addition
to this display gen number.
Lucas De Marchi
>
> 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. ;)
>
>
> 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
More information about the Intel-gfx
mailing list