[Intel-gfx] [PATCH v2] drm/i915: Always program CSR if CSR is uninitialized

[Patrik Jakobsson]

On Sat, Oct 24, 2015 at 11:03:05AM +0530, Animesh Manna wrote:
> 
> 
> On 10/23/2015 3:11 PM, Patrik Jakobsson wrote:
> >The current CSR loading code depends on the CSR program memory to be
> >cleared after boot. This is unfortunately not true on all hardware.
> >Instead make use of the FW_UNINITIALIZED state in init and check for
> >FW_LOADED to prevent init path from skipping the actual programming.
> >
> >v2: Move initialization of state to after HAS_CSR() check
> >
> >Signed-off-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> >Tested-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_csr.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> >index 9e530a7..e74c09e 100644
> >--- a/drivers/gpu/drm/i915/intel_csr.c
> >+++ b/drivers/gpu/drm/i915/intel_csr.c
> >@@ -271,7 +271,7 @@ void intel_csr_load_program(struct drm_device *dev)
> >  	 * Unfortunately the ACPI subsystem doesn't yet give us a way to
> >  	 * differentiate this, hence figure it out with this hack.
> >  	 */
> >-	if (I915_READ(CSR_PROGRAM(0)))
> >+	if (I915_READ(CSR_PROGRAM(0)) && dev_priv->csr.state == FW_LOADED).state
> 
> As I will be removing csr.state in dmc-redesign patch series my suggestion would be to
> compare register read with first element of payload like below:

Is the redesign series likely to land soon? Currently the dmc fw isn't loading
at all on skl which is a stopper for quite a few features.

> if (I915_READ(CSR_PROGRAM(0)) == dmc_payload[0]) and then skip the f/w loading part.

I haven't looked at what is stored in CSR_PROGRAM(0) on skl after boot but can
we really trust this check? Is there really no other way to verify if it's up
and running already? What was the conclusion from your discussion with Daniel
about keeping track of this manually?

-Patrik

> -Animesh
> 
> 
> >  		return;
> >  	mutex_lock(&dev_priv->csr_lock);
> >@@ -428,6 +428,8 @@ void intel_csr_ucode_init(struct drm_device *dev)
> >  	if (!HAS_CSR(dev))
> >  		return;
> >+	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
> >+
> >  	if (IS_SKYLAKE(dev))
> >  		csr->fw_path = I915_CSR_SKL;
> >  	else if (IS_BROXTON(dev_priv))
>