On 12/17/2015 07:14 AM, Mika Kuoppala wrote:
> Since commit 940aece471bd ("drm/i915/vlv: Valleyview support
> for forcewake Individual power wells.") we have only taken
> media engine forcewake correctly on reads, but only taken render
> engine forcewake on media engine writes and omitted the media
> domain.
>
> This asymmetry might have caused unstable behaviour on
> media ring access.
>
> Fix is to take media engine forcewake symmetrically to writes.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=88012
> Cc: Deepak S <deepak.s@xxxxxxxxx>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 277e60ae0e47..a2e204088aa5 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -902,6 +902,23 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
> GEN6_WRITE_FOOTER; \
> }
>
> +#define __vlv_write(x) \
> +static void \
> +vlv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
> + enum forcewake_domains fw_engine = 0; \
> + GEN6_WRITE_HEADER; \
> + if (!NEEDS_FORCE_WAKE(offset)) \
> + fw_engine = 0; \
> + else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_RENDER; \
> + else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_MEDIA; \
> + if (fw_engine) \
> + __force_wake_get(dev_priv, fw_engine); \
> + __raw_i915_write##x(dev_priv, reg, val); \
> + GEN6_WRITE_FOOTER; \
> +}
> +
> static const i915_reg_t gen8_shadowed_regs[] = {
> FORCEWAKE_MT,
> GEN6_RPNSWREQ,
> @@ -1019,6 +1036,10 @@ __gen8_write(8)
> __gen8_write(16)
> __gen8_write(32)
> __gen8_write(64)
> +__vlv_write(8)
> +__vlv_write(16)
> +__vlv_write(32)
> +__vlv_write(64)
> __hsw_write(8)
> __hsw_write(16)
> __hsw_write(32)
> @@ -1031,6 +1052,7 @@ __gen6_write(64)
> #undef __gen9_write
> #undef __chv_write
> #undef __gen8_write
> +#undef __vlv_write
> #undef __hsw_write
> #undef __gen6_write
> #undef GEN6_WRITE_FOOTER
> @@ -1243,6 +1265,8 @@ void intel_uncore_init(struct drm_device *dev)
> case 6:
> if (IS_HASWELL(dev)) {
> ASSIGN_WRITE_MMIO_VFUNCS(hsw);
> + } else if (IS_VALLEYVIEW(dev)) {
> + ASSIGN_WRITE_MMIO_VFUNCS(vlv);
> } else {
> ASSIGN_WRITE_MMIO_VFUNCS(gen6);
> }
>
Looks good. Looks like we also have it on chv, so I guess it was just
an oversight.
Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
