| Message ID | 20190322002431.9585-1-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: stop storing the media fuse | expand |
Quoting Daniele Ceraolo Spurio (2019-03-22 00:24:31) > We're already updating the engine_mask to reflect what's in the HW, so > we can just get the info from there. A couple of macros have been added > to facilitate this. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ > drivers/gpu/drm/i915/intel_device_info.c | 15 ++++++++------- > drivers/gpu/drm/i915/intel_device_info.h | 4 ---- > 3 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index fefcb39aefc4..9d8d423641d2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2440,6 +2440,12 @@ static inline unsigned int i915_sg_segment_size(void) > #define ALL_ENGINES (~0u) > #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask & BIT(id)) > > +#define ENGINE_INSTANCES_MASK(dev_priv, first, count) \ > + ((INTEL_INFO(dev_priv)->engine_mask & \ > + GENMASK(first + count - 1, first)) >> first) Checkpatch complains if we don't wrap everything (inside). We could even go so far as ({ int first__ = (first); int count__ = (count); INTEL_INFO(i915)-engine_mask & GENMASK(first__ + count__ - 1, first__) >> first__; }) gcc should be smart enough to constant fold that away. > +#define VDBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VCS0, I915_MAX_VCS); > +#define VEBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VECS0, I915_MAX_VECS); > + > #define HAS_LLC(dev_priv) (INTEL_INFO(dev_priv)->has_llc) > #define HAS_SNOOP(dev_priv) (INTEL_INFO(dev_priv)->has_snoop) > #define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & EDRAM_ENABLED)) > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index eddf83807957..74efabd12351 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -871,22 +871,23 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) > unsigned int logical_vdbox = 0; > unsigned int i; > u32 media_fuse; > + u16 vdbox_mask, vebox_mask; I would put these on separate lines, just to keep the vertical whitespace clean. > if (INTEL_GEN(dev_priv) < 11) > return; > > media_fuse = ~I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE); > > - RUNTIME_INFO(dev_priv)->vdbox_enable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; > - RUNTIME_INFO(dev_priv)->vebox_enable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> > - GEN11_GT_VEBOX_DISABLE_SHIFT; > + vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; > + vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> > + GEN11_GT_VEBOX_DISABLE_SHIFT; > > - DRM_DEBUG_DRIVER("vdbox enable: %04x\n", RUNTIME_INFO(dev_priv)->vdbox_enable); > + DRM_DEBUG_DRIVER("vdbox enable: %04x\n", vdbox_mask); > for (i = 0; i < I915_MAX_VCS; i++) { > if (!HAS_ENGINE(dev_priv, _VCS(i))) > continue; > > - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vdbox_enable)) { > + if (!(BIT(i) & vdbox_mask)) { > info->engine_mask &= ~BIT(_VCS(i)); > DRM_DEBUG_DRIVER("vcs%u fused off\n", i); > continue; > @@ -900,12 +901,12 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) > RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= BIT(i); > } > > - DRM_DEBUG_DRIVER("vebox enable: %04x\n", RUNTIME_INFO(dev_priv)->vebox_enable); > + DRM_DEBUG_DRIVER("vebox enable: %04x\n", vebox_mask); > for (i = 0; i < I915_MAX_VECS; i++) { > if (!HAS_ENGINE(dev_priv, _VECS(i))) > continue; > > - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vebox_enable)) { > + if (!(BIT(i) & vebox_mask)) { > info->engine_mask &= ~BIT(_VECS(i)); > DRM_DEBUG_DRIVER("vecs%u fused off\n", i); > } > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 6234570a9b17..06f428c70ff5 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -208,10 +208,6 @@ struct intel_runtime_info { > > u32 cs_timestamp_frequency_khz; > > - /* Enabled (not fused off) media engine bitmasks. */ > - u8 vdbox_enable; > - u8 vebox_enable; > - > /* Media engine access to SFC per instance */ > u8 vdbox_sfc_access; Code looks ok, just checkpatch nits Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Michal, as the most likely user, what do you think? -Chris
On 22/03/2019 00:24, Daniele Ceraolo Spurio wrote: > We're already updating the engine_mask to reflect what's in the HW, so > we can just get the info from there. A couple of macros have been added > to facilitate this. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ > drivers/gpu/drm/i915/intel_device_info.c | 15 ++++++++------- > drivers/gpu/drm/i915/intel_device_info.h | 4 ---- > 3 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index fefcb39aefc4..9d8d423641d2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2440,6 +2440,12 @@ static inline unsigned int i915_sg_segment_size(void) > #define ALL_ENGINES (~0u) > #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask & BIT(id)) > > +#define ENGINE_INSTANCES_MASK(dev_priv, first, count) \ > + ((INTEL_INFO(dev_priv)->engine_mask & \ > + GENMASK(first + count - 1, first)) >> first) > +#define VDBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VCS0, I915_MAX_VCS); > +#define VEBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VECS0, I915_MAX_VECS); Are we confident the existing commentary against enum intel_engine_id (keep instances of a class together) is enough to keep this working forevermore or more paranoia could be useful? Regards, Tvrtko > + > #define HAS_LLC(dev_priv) (INTEL_INFO(dev_priv)->has_llc) > #define HAS_SNOOP(dev_priv) (INTEL_INFO(dev_priv)->has_snoop) > #define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & EDRAM_ENABLED)) > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index eddf83807957..74efabd12351 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -871,22 +871,23 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) > unsigned int logical_vdbox = 0; > unsigned int i; > u32 media_fuse; > + u16 vdbox_mask, vebox_mask; > > if (INTEL_GEN(dev_priv) < 11) > return; > > media_fuse = ~I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE); > > - RUNTIME_INFO(dev_priv)->vdbox_enable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; > - RUNTIME_INFO(dev_priv)->vebox_enable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> > - GEN11_GT_VEBOX_DISABLE_SHIFT; > + vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; > + vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> > + GEN11_GT_VEBOX_DISABLE_SHIFT; > > - DRM_DEBUG_DRIVER("vdbox enable: %04x\n", RUNTIME_INFO(dev_priv)->vdbox_enable); > + DRM_DEBUG_DRIVER("vdbox enable: %04x\n", vdbox_mask); > for (i = 0; i < I915_MAX_VCS; i++) { > if (!HAS_ENGINE(dev_priv, _VCS(i))) > continue; > > - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vdbox_enable)) { > + if (!(BIT(i) & vdbox_mask)) { > info->engine_mask &= ~BIT(_VCS(i)); > DRM_DEBUG_DRIVER("vcs%u fused off\n", i); > continue; > @@ -900,12 +901,12 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) > RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= BIT(i); > } > > - DRM_DEBUG_DRIVER("vebox enable: %04x\n", RUNTIME_INFO(dev_priv)->vebox_enable); > + DRM_DEBUG_DRIVER("vebox enable: %04x\n", vebox_mask); > for (i = 0; i < I915_MAX_VECS; i++) { > if (!HAS_ENGINE(dev_priv, _VECS(i))) > continue; > > - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vebox_enable)) { > + if (!(BIT(i) & vebox_mask)) { > info->engine_mask &= ~BIT(_VECS(i)); > DRM_DEBUG_DRIVER("vecs%u fused off\n", i); > } > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 6234570a9b17..06f428c70ff5 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -208,10 +208,6 @@ struct intel_runtime_info { > > u32 cs_timestamp_frequency_khz; > > - /* Enabled (not fused off) media engine bitmasks. */ > - u8 vdbox_enable; > - u8 vebox_enable; > - > /* Media engine access to SFC per instance */ > u8 vdbox_sfc_access; > }; >
On 22/03/2019 09:23, Tvrtko Ursulin wrote: > > On 22/03/2019 00:24, Daniele Ceraolo Spurio wrote: >> We're already updating the engine_mask to reflect what's in the HW, so >> we can just get the info from there. A couple of macros have been added >> to facilitate this. >> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ >> drivers/gpu/drm/i915/intel_device_info.c | 15 ++++++++------- >> drivers/gpu/drm/i915/intel_device_info.h | 4 ---- >> 3 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index fefcb39aefc4..9d8d423641d2 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2440,6 +2440,12 @@ static inline unsigned int >> i915_sg_segment_size(void) >> #define ALL_ENGINES (~0u) >> #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask >> & BIT(id)) >> +#define ENGINE_INSTANCES_MASK(dev_priv, first, count) \ >> + ((INTEL_INFO(dev_priv)->engine_mask & \ >> + GENMASK(first + count - 1, first)) >> first) >> +#define VDBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VCS0, >> I915_MAX_VCS); >> +#define VEBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VECS0, >> I915_MAX_VECS); > > Are we confident the existing commentary against enum intel_engine_id > (keep instances of a class together) is enough to keep this working > forevermore or more paranoia could be useful? Same actually applies to existing _V(E)CS macros so I guess it is okay. I would have been quickly notice anyway if unexpected engine went missing. :) > Regards, > > Tvrtko > >> + >> #define HAS_LLC(dev_priv) (INTEL_INFO(dev_priv)->has_llc) >> #define HAS_SNOOP(dev_priv) (INTEL_INFO(dev_priv)->has_snoop) >> #define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & >> EDRAM_ENABLED)) >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c >> b/drivers/gpu/drm/i915/intel_device_info.c >> index eddf83807957..74efabd12351 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -871,22 +871,23 @@ void intel_device_info_init_mmio(struct >> drm_i915_private *dev_priv) >> unsigned int logical_vdbox = 0; >> unsigned int i; >> u32 media_fuse; >> + u16 vdbox_mask, vebox_mask; >> if (INTEL_GEN(dev_priv) < 11) >> return; >> media_fuse = ~I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE); >> - RUNTIME_INFO(dev_priv)->vdbox_enable = media_fuse & >> GEN11_GT_VDBOX_DISABLE_MASK; >> - RUNTIME_INFO(dev_priv)->vebox_enable = (media_fuse & >> GEN11_GT_VEBOX_DISABLE_MASK) >> >> - GEN11_GT_VEBOX_DISABLE_SHIFT; >> + vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; >> + vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> >> + GEN11_GT_VEBOX_DISABLE_SHIFT; >> - DRM_DEBUG_DRIVER("vdbox enable: %04x\n", >> RUNTIME_INFO(dev_priv)->vdbox_enable); >> + DRM_DEBUG_DRIVER("vdbox enable: %04x\n", vdbox_mask); >> for (i = 0; i < I915_MAX_VCS; i++) { >> if (!HAS_ENGINE(dev_priv, _VCS(i))) >> continue; >> - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vdbox_enable)) { >> + if (!(BIT(i) & vdbox_mask)) { >> info->engine_mask &= ~BIT(_VCS(i)); >> DRM_DEBUG_DRIVER("vcs%u fused off\n", i); >> continue; >> @@ -900,12 +901,12 @@ void intel_device_info_init_mmio(struct >> drm_i915_private *dev_priv) >> RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= BIT(i); >> } >> - DRM_DEBUG_DRIVER("vebox enable: %04x\n", >> RUNTIME_INFO(dev_priv)->vebox_enable); >> + DRM_DEBUG_DRIVER("vebox enable: %04x\n", vebox_mask); >> for (i = 0; i < I915_MAX_VECS; i++) { >> if (!HAS_ENGINE(dev_priv, _VECS(i))) >> continue; >> - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vebox_enable)) { >> + if (!(BIT(i) & vebox_mask)) { >> info->engine_mask &= ~BIT(_VECS(i)); >> DRM_DEBUG_DRIVER("vecs%u fused off\n", i); >> } >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h >> b/drivers/gpu/drm/i915/intel_device_info.h >> index 6234570a9b17..06f428c70ff5 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.h >> +++ b/drivers/gpu/drm/i915/intel_device_info.h >> @@ -208,10 +208,6 @@ struct intel_runtime_info { >> u32 cs_timestamp_frequency_khz; >> - /* Enabled (not fused off) media engine bitmasks. */ >> - u8 vdbox_enable; >> - u8 vebox_enable; >> - >> /* Media engine access to SFC per instance */ >> u8 vdbox_sfc_access; >> }; >>
On Fri, 22 Mar 2019 09:29:58 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Daniele Ceraolo Spurio (2019-03-22 00:24:31) >> We're already updating the engine_mask to reflect what's in the HW, so >> we can just get the info from there. A couple of macros have been added >> to facilitate this. >> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ >> drivers/gpu/drm/i915/intel_device_info.c | 15 ++++++++------- >> drivers/gpu/drm/i915/intel_device_info.h | 4 ---- >> 3 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index fefcb39aefc4..9d8d423641d2 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2440,6 +2440,12 @@ static inline unsigned int >> i915_sg_segment_size(void) >> #define ALL_ENGINES (~0u) >> #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask & >> BIT(id)) >> >> +#define ENGINE_INSTANCES_MASK(dev_priv, first, count) \ >> + ((INTEL_INFO(dev_priv)->engine_mask & \ >> + GENMASK(first + count - 1, first)) >> first) > > Checkpatch complains if we don't wrap everything (inside). > > We could even go so far as > > ({ > int first__ = (first); > int count__ = (count); > INTEL_INFO(i915)-engine_mask & GENMASK(first__ + count__ - 1, first__) > >> first__; > }) > > gcc should be smart enough to constant fold that away. > >> +#define VDBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VCS0, >> I915_MAX_VCS); >> +#define VEBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VECS0, >> I915_MAX_VECS); >> + >> #define HAS_LLC(dev_priv) (INTEL_INFO(dev_priv)->has_llc) >> #define HAS_SNOOP(dev_priv) (INTEL_INFO(dev_priv)->has_snoop) >> #define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & >> EDRAM_ENABLED)) >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c >> b/drivers/gpu/drm/i915/intel_device_info.c >> index eddf83807957..74efabd12351 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -871,22 +871,23 @@ void intel_device_info_init_mmio(struct >> drm_i915_private *dev_priv) >> unsigned int logical_vdbox = 0; >> unsigned int i; >> u32 media_fuse; >> + u16 vdbox_mask, vebox_mask; > > I would put these on separate lines, just to keep the vertical > whitespace clean. > >> if (INTEL_GEN(dev_priv) < 11) >> return; >> >> media_fuse = ~I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE); >> >> - RUNTIME_INFO(dev_priv)->vdbox_enable = media_fuse & >> GEN11_GT_VDBOX_DISABLE_MASK; >> - RUNTIME_INFO(dev_priv)->vebox_enable = (media_fuse & >> GEN11_GT_VEBOX_DISABLE_MASK) >> >> - GEN11_GT_VEBOX_DISABLE_SHIFT; >> + vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; >> + vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> >> + GEN11_GT_VEBOX_DISABLE_SHIFT; >> >> - DRM_DEBUG_DRIVER("vdbox enable: %04x\n", >> RUNTIME_INFO(dev_priv)->vdbox_enable); >> + DRM_DEBUG_DRIVER("vdbox enable: %04x\n", vdbox_mask); >> for (i = 0; i < I915_MAX_VCS; i++) { >> if (!HAS_ENGINE(dev_priv, _VCS(i))) >> continue; >> >> - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vdbox_enable)) { >> + if (!(BIT(i) & vdbox_mask)) { >> info->engine_mask &= ~BIT(_VCS(i)); >> DRM_DEBUG_DRIVER("vcs%u fused off\n", i); >> continue; >> @@ -900,12 +901,12 @@ void intel_device_info_init_mmio(struct >> drm_i915_private *dev_priv) >> RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= >> BIT(i); >> } >> >> - DRM_DEBUG_DRIVER("vebox enable: %04x\n", >> RUNTIME_INFO(dev_priv)->vebox_enable); >> + DRM_DEBUG_DRIVER("vebox enable: %04x\n", vebox_mask); >> for (i = 0; i < I915_MAX_VECS; i++) { >> if (!HAS_ENGINE(dev_priv, _VECS(i))) >> continue; >> >> - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vebox_enable)) { >> + if (!(BIT(i) & vebox_mask)) { >> info->engine_mask &= ~BIT(_VECS(i)); >> DRM_DEBUG_DRIVER("vecs%u fused off\n", i); >> } >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h >> b/drivers/gpu/drm/i915/intel_device_info.h >> index 6234570a9b17..06f428c70ff5 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.h >> +++ b/drivers/gpu/drm/i915/intel_device_info.h >> @@ -208,10 +208,6 @@ struct intel_runtime_info { >> >> u32 cs_timestamp_frequency_khz; >> >> - /* Enabled (not fused off) media engine bitmasks. */ >> - u8 vdbox_enable; >> - u8 vebox_enable; >> - >> /* Media engine access to SFC per instance */ >> u8 vdbox_sfc_access; > > Code looks ok, just checkpatch nits > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Michal, as the most likely user, what do you think? > -Chris Can't spot any other nits, Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> ~Michal
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fefcb39aefc4..9d8d423641d2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2440,6 +2440,12 @@ static inline unsigned int i915_sg_segment_size(void) #define ALL_ENGINES (~0u) #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask & BIT(id)) +#define ENGINE_INSTANCES_MASK(dev_priv, first, count) \ + ((INTEL_INFO(dev_priv)->engine_mask & \ + GENMASK(first + count - 1, first)) >> first) +#define VDBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VCS0, I915_MAX_VCS); +#define VEBOX_MASK(dev_priv) ENGINE_INSTANCES_MASK(dev_priv, VECS0, I915_MAX_VECS); + #define HAS_LLC(dev_priv) (INTEL_INFO(dev_priv)->has_llc) #define HAS_SNOOP(dev_priv) (INTEL_INFO(dev_priv)->has_snoop) #define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & EDRAM_ENABLED)) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index eddf83807957..74efabd12351 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -871,22 +871,23 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) unsigned int logical_vdbox = 0; unsigned int i; u32 media_fuse; + u16 vdbox_mask, vebox_mask; if (INTEL_GEN(dev_priv) < 11) return; media_fuse = ~I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE); - RUNTIME_INFO(dev_priv)->vdbox_enable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; - RUNTIME_INFO(dev_priv)->vebox_enable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> - GEN11_GT_VEBOX_DISABLE_SHIFT; + vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK; + vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >> + GEN11_GT_VEBOX_DISABLE_SHIFT; - DRM_DEBUG_DRIVER("vdbox enable: %04x\n", RUNTIME_INFO(dev_priv)->vdbox_enable); + DRM_DEBUG_DRIVER("vdbox enable: %04x\n", vdbox_mask); for (i = 0; i < I915_MAX_VCS; i++) { if (!HAS_ENGINE(dev_priv, _VCS(i))) continue; - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vdbox_enable)) { + if (!(BIT(i) & vdbox_mask)) { info->engine_mask &= ~BIT(_VCS(i)); DRM_DEBUG_DRIVER("vcs%u fused off\n", i); continue; @@ -900,12 +901,12 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv) RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= BIT(i); } - DRM_DEBUG_DRIVER("vebox enable: %04x\n", RUNTIME_INFO(dev_priv)->vebox_enable); + DRM_DEBUG_DRIVER("vebox enable: %04x\n", vebox_mask); for (i = 0; i < I915_MAX_VECS; i++) { if (!HAS_ENGINE(dev_priv, _VECS(i))) continue; - if (!(BIT(i) & RUNTIME_INFO(dev_priv)->vebox_enable)) { + if (!(BIT(i) & vebox_mask)) { info->engine_mask &= ~BIT(_VECS(i)); DRM_DEBUG_DRIVER("vecs%u fused off\n", i); } diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 6234570a9b17..06f428c70ff5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -208,10 +208,6 @@ struct intel_runtime_info { u32 cs_timestamp_frequency_khz; - /* Enabled (not fused off) media engine bitmasks. */ - u8 vdbox_enable; - u8 vebox_enable; - /* Media engine access to SFC per instance */ u8 vdbox_sfc_access; };
We're already updating the engine_mask to reflect what's in the HW, so we can just get the info from there. A couple of macros have been added to facilitate this. Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ drivers/gpu/drm/i915/intel_device_info.c | 15 ++++++++------- drivers/gpu/drm/i915/intel_device_info.h | 4 ---- 3 files changed, 14 insertions(+), 11 deletions(-)