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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxx>
Michal, as the most likely user, what do you think?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
