Re: [Intel-gfx] [PATCH] drm/i915/psr: Force PSR probe only after full initialization

From: Ross Zwisler <zwisler@google.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/psr: Force PSR probe only after full initialization
Date: Fri, 14 Feb 2020 11:14:40 -0700	[thread overview]
Message-ID: <20200214181440.GA24285@google.com> (raw)
In-Reply-To: <20200214015038.122913-1-jose.souza@intel.com>

On Thu, Feb 13, 2020 at 05:50:38PM -0800, José Roberto de Souza wrote:
> Commit 60c6a14b489b ("drm/i915/display: Force the state compute phase
> once to enable PSR") was forcing the state compute too earlier
> causing errors because not everything was initialized, so here
> moving to i915_driver_register() when everything is ready and driver
> is registering into the rest of the system.
> 
> Also fixing the place where it disarm the force probe as during the
> atomic check phase errors could happen like the ones due locking and
> it would cause PSR to never be enabled if that happens.
> Leaving the disarm to the atomic commit phase, intel_psr_enable() or
> intel_psr_update() will be called even if the current state do not
> allow PSR to be enabled.
> 
> Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute phase once to enable PSR")
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> Reported-by: Ross Zwisler <zwisler@google.com>

Tested-by: Ross Zwisler <zwisler@google.com>

> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Note that when applying to the current upstream/master from Linus you have one
small conflict:

$ cat drivers/gpu/drm/i915/i915_drv.h.rej

--- drivers/gpu/drm/i915/i915_drv.h
+++ drivers/gpu/drm/i915/i915_drv.h
@@ -505,7 +505,7 @@ struct i915_psr {
 	bool dc3co_enabled;
 	u32 dc3co_exit_delay;
 	struct delayed_work dc3co_work;
-	bool initially_probed;
+	bool force_mode_changed;
 };

In Linus's tree the end of that structure looks like:

	bool dc3co_enabled;
	u32 dc3co_exit_delay;
	struct delayed_work idle_work;
	bool initially_probed;
};

Where the 'struct delayed_work' element is named differently.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
