On Wed, 2019-07-17 at 19:06 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Instead of re-calculating the MCR selector in read_subslice_reg do
> the
> rwm on its existing value and restore it when done.
>
> This consolidates MCR programming to one place for cnl+, and avoids
> re-calculating its default value on older platforms during hangcheck.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 37 ++++++++-------------
> --
> 1 file changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 6f93caf7a5a1..cc4d1826173d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -989,27 +989,17 @@ read_subslice_reg(struct intel_engine_cs
> *engine, int slice, int subslice,
> {
> struct drm_i915_private *i915 = engine->i915;
> struct intel_uncore *uncore = engine->uncore;
> - u32 mcr_slice_subslice_mask;
> - u32 mcr_slice_subslice_select;
> - u32 default_mcr_s_ss_select;
> - u32 mcr;
> - u32 ret;
> + u32 mcr_mask, mcr_ss, mcr, old_mcr, val;
> enum forcewake_domains fw_domains;
>
> if (INTEL_GEN(i915) >= 11) {
> - mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK |
> - GEN11_MCR_SUBSLICE_MASK;
> - mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) |
> - GEN11_MCR_SUBSLICE(subslice
> );
> + mcr_mask = GEN11_MCR_SLICE_MASK |
> GEN11_MCR_SUBSLICE_MASK;
> + mcr_ss = GEN11_MCR_SLICE(slice) |
> GEN11_MCR_SUBSLICE(subslice);
> } else {
> - mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK |
> - GEN8_MCR_SUBSLICE_MASK;
> - mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) |
> - GEN8_MCR_SUBSLICE(subslice)
> ;
> + mcr_mask = GEN8_MCR_SLICE_MASK |
> GEN8_MCR_SUBSLICE_MASK;
> + mcr_ss = GEN8_MCR_SLICE(slice) |
> GEN8_MCR_SUBSLICE(subslice);
> }
>
> - default_mcr_s_ss_select =
> intel_calculate_mcr_s_ss_select(i915);
I'm only now reading one user of this function after this change. Does
it make sense to move it to gt/intel_workarounds.c as a static function
instead? Not critical here.
Reviewed-by: Stuart Summers <stuart.summers@xxxxxxxxx>
> -
> fw_domains = intel_uncore_forcewake_for_reg(uncore, reg,
> FW_REG_READ);
> fw_domains |= intel_uncore_forcewake_for_reg(uncore,
> @@ -1019,26 +1009,23 @@ read_subslice_reg(struct intel_engine_cs
> *engine, int slice, int subslice,
> spin_lock_irq(&uncore->lock);
> intel_uncore_forcewake_get__locked(uncore, fw_domains);
>
> - mcr = intel_uncore_read_fw(uncore, GEN8_MCR_SELECTOR);
> -
> - WARN_ON_ONCE((mcr & mcr_slice_subslice_mask) !=
> - default_mcr_s_ss_select);
> + old_mcr = mcr = intel_uncore_read_fw(uncore,
> GEN8_MCR_SELECTOR);
>
> - mcr &= ~mcr_slice_subslice_mask;
> - mcr |= mcr_slice_subslice_select;
> + mcr &= ~mcr_mask;
> + mcr |= mcr_ss;
> intel_uncore_write_fw(uncore, GEN8_MCR_SELECTOR, mcr);
>
> - ret = intel_uncore_read_fw(uncore, reg);
> + val = intel_uncore_read_fw(uncore, reg);
>
> - mcr &= ~mcr_slice_subslice_mask;
> - mcr |= default_mcr_s_ss_select;
> + mcr &= ~mcr_mask;
> + mcr |= old_mcr & mcr_mask;
>
> intel_uncore_write_fw(uncore, GEN8_MCR_SELECTOR, mcr);
>
> intel_uncore_forcewake_put__locked(uncore, fw_domains);
> spin_unlock_irq(&uncore->lock);
>
> - return ret;
> + return val;
> }
>
> /* NB: please notice the memset */
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
