On Mon, Dec 19, 2022 at 06:29:27PM +0100, Sasa Dragic wrote:
> RC6p on Sandy Bridge got re-enabled over time, causing visual glitches
> and GPU hangs.
> 
> Disabled originally in commit 1c8ecf80fdee ("drm/i915: do not enable
> RC6p on Sandy Bridge").

re cover letter:
> It was originally disabled in commit 1c8ecf80fdee ("drm/i915: do not
> enable RC6p on Sandy Bridge"), and subsequently re-enabled by
> 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode
> for rc6"), which seems to focus only on Ivy Bridge.

That only kicks in while parked (ie. fully idle from
software POV). I think the real bad commit is
fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
which seems to affects which rc6 level is selected while
unparked.

We should mention both of those commits here:
Fixes: fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
Fixes: 13c5a577b342 ("drm/i915/gt: Select the deepest available parking mode 
for rc6")

Also we want
Cc: sta...@vger.kernel.org

We can add those while pushing, so no need to resend for that.

> 
> Signed-off-by: Sasa Dragic <sasa.dra...@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 668e9da52584..69377564028a 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -423,7 +423,8 @@ static const struct intel_device_info ilk_m_info = {
>       .has_coherent_ggtt = true, \
>       .has_llc = 1, \
>       .has_rc6 = 1, \
> -     .has_rc6p = 1, \
> +     /* snb does support rc6p, but enabling it causes various issues */ \
> +     .has_rc6p = 0, \

The one downside of doing it this way is that we also lose
the debugfs/sysfs files so we can't monitor rc6+/++
residency anymore to make sure they are not entered.

I think ideally we'd split this into two parts: which rc6
states the hw actually has vs. which rc6 states we actually
want to use. But at least for the time being I think this
simple should be sufficient, and should be easy to backport
to stable releases.

>       .has_rps = true, \
>       .dma_mask_size = 40, \
>       .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, \
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel

Reply via email to