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.v...@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.v...@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@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx