[DMC_REDESIGN_V2 00/14] Redesign dmc firmware loading.

[DMC_REDESIGN_V2 00/14] Redesign dmc firmware loading.

[Animesh Manna]

This patch series has the changes done to redesign the dmc firmware
loading flow. This is continuation of the below patch series after
addressing review comments from Daniel.

v1: http://lists.freedesktop.org/archives/intel-gfx/2015-August/072921.html

v2: 
- After rebasing based on below patch series.
http://lists.freedesktop.org/archives/intel-gfx/2015-August/072870.html
- Two more patches added in this patch series from older patch series.
http://lists.freedesktop.org/archives/intel-gfx/2015-July/071163.html

Animesh Manna (5):
  drm/i915/gen9: csr_init after runtime pm enable
  drm/i915/bxt: release rpm reference if csr firmware failed to load.
  drm/i915/gen9: Use flush_work to synchronize with dmc loader
  drm/i915/skl: Removed assert for csr-fw-loading check during disabling
    dc6
  drm/i915/gen9: Corrected the sanity check of mmio address range for
    csr.

Daniel Vetter (9):
  drm/i915: use correct power domain for csr loading
  drm/i915/gen9: move assert_csr_loaded into intel_rpm.c
  drm/i915/gen9: Remove csr.state, csr_lock and related code.
  drm/i915/gen9: Align line continuations in intel_csr.c.
  drm/i915/gen9: Simplify csr loading failure printing.
  drm/i915/gen9: extract parse_csr_fw
  drm/i915/gen9: Don't try to load garbage dmc firmware on resume
  drm/i915/gen9: Use dev_priv in csr functions
  drm/i915: Use request_firmware and our own async work

 drivers/gpu/drm/i915/i915_dma.c         |  11 +-
 drivers/gpu/drm/i915/i915_drv.c         |  39 +-----
 drivers/gpu/drm/i915/i915_drv.h         |  12 +-
 drivers/gpu/drm/i915/i915_reg.h         |  16 +++
 drivers/gpu/drm/i915/intel_csr.c        | 211 ++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h        |  10 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c |  26 ++--
 7 files changed, 114 insertions(+), 211 deletions(-)

-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 01/14] drm/i915/gen9: csr_init after runtime pm enable

[Animesh Manna]

Skl is fully dependent on dmc for going to low power state (dc5/dc6).
This requires a trigger from rpm. To ensure the dmc firmware
is available for runtime pm support rpm-reference-count is used
by not releasing the rpm reference if firmware loading is
not completed.

So moved the intel_csr_ucode_init call after runtime pm enable.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2193cc2..48b9792 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -885,9 +885,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_uncore_init(dev);
 
-	/* Load CSR Firmware for SKL */
-	intel_csr_ucode_init(dev);
-
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
 		goto out_freecsr;
@@ -1035,6 +1032,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	i915_audio_component_init(dev_priv);
 
+	/* Load CSR Firmware for SKL */
+	intel_csr_ucode_init(dev);
+
 	return 0;
 
 out_power_well:
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 01/14] drm/i915/gen9: csr_init after runtime pm enable

[Imre Deak]

On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> Skl is fully dependent on dmc for going to low power state (dc5/dc6).
> This requires a trigger from rpm. To ensure the dmc firmware
> is available for runtime pm support rpm-reference-count is used
> by not releasing the rpm reference if firmware loading is
> not completed.

The above doesn't explain to me why we need this change. Looking at the
next patch makes it clearer: we need to move the firmware loading later
since it needs to take a power domain reference instead of an RPM
reference, and power domains are not initialized at this point yet. It's
also worth mentioning in the commit log that this change is needed by an
upcoming patch.

> 
> So moved the intel_csr_ucode_init call after runtime pm enable.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2193cc2..48b9792 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -885,9 +885,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	intel_uncore_init(dev);
>  
> -	/* Load CSR Firmware for SKL */
> -	intel_csr_ucode_init(dev);
> -
>  	ret = i915_gem_gtt_init(dev);
>  	if (ret)
>  		goto out_freecsr;
> @@ -1035,6 +1032,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	i915_audio_component_init(dev_priv);
>  
> +	/* Load CSR Firmware for SKL */
> +	intel_csr_ucode_init(dev);

We need to call this function earlier, since there could be a modeset
before this for the console, after which the driver will disable any
unneeded power domains. So this should be called after
intel_power_domains_init_hw() to prevent this.

> +
>  	return 0;
>  
>  out_power_well:


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 02/14] drm/i915: use correct power domain for csr loading

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Grabbing a runtime pm reference with intel_runtime_pm_get will only
prevent device D3. But dmc firmware is required even earlier (namely
for the skl power well 2).

Hence we need to grab a rpm reference higher up in the hierarchy. For
simplicity just grab the _INIT display power well. That's a bit too
much, but since the firmware loading task should completely fairly
quickly this won't be a real problem really.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 682cc26..75a775b 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -393,7 +393,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
 out:
 	if (fw_loaded)
-		intel_runtime_pm_put(dev_priv);
+		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 	else
 		intel_csr_load_status_set(dev_priv, FW_FAILED);
 
@@ -430,7 +430,7 @@ void intel_csr_ucode_init(struct drm_device *dev)
 	 * Obtain a runtime pm reference, until CSR is loaded,
 	 * to avoid entering runtime-suspend.
 	 */
-	intel_runtime_pm_get(dev_priv);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
 	/* CSR supported for platform, load firmware */
 	ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path,
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 03/14] drm/i915/bxt: release rpm reference if csr firmware failed to load.

[Animesh Manna]

Note that for bxt without dmc, display engine can go to lowest
possible state (dc9), so releasing the rpm reference.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 75a775b..be388da 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -392,7 +392,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 
 	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
 out:
-	if (fw_loaded)
+	if (fw_loaded || IS_BROXTON(dev))
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 	else
 		intel_csr_load_status_set(dev_priv, FW_FAILED);
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 03/14] drm/i915/bxt: release rpm reference if csr firmware failed to load.

[Imre Deak]

On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> Note that for bxt without dmc, display engine can go to lowest
> possible state (dc9), so releasing the rpm reference.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 75a775b..be388da 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -392,7 +392,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  
>  	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
>  out:
> -	if (fw_loaded)
> +	if (fw_loaded || IS_BROXTON(dev))
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);

I don't think this is needed. We disable all display side runtime power
management if the firmware is not available. So no need to special case
this either imo.

>  	else
>  		intel_csr_load_status_set(dev_priv, FW_FAILED);


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 03/14] drm/i915/bxt: release rpm reference if csr firmware failed to load.

[Animesh Manna]

On 9/30/2015 7:54 PM, Imre Deak wrote:
> On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
>> Note that for bxt without dmc, display engine can go to lowest
>> possible state (dc9), so releasing the rpm reference.
>>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Damien Lespiau <damien.lespiau@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Sunil Kamath <sunil.kamath@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_csr.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> index 75a775b..be388da 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -392,7 +392,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>>   
>>   	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
>>   out:
>> -	if (fw_loaded)
>> +	if (fw_loaded || IS_BROXTON(dev))
>>   		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> I don't think this is needed. We disable all display side runtime power
> management if the firmware is not available. So no need to special case
> this either imo.
In display off case for broxton platform we can directly goto dc9 state 
for which dmc is not needed.
So, don't want to block runtime power management from display side.

-Animesh
>
>>   	else
>>   		intel_csr_load_status_set(dev_priv, FW_FAILED);
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 03/14] drm/i915/bxt: release rpm reference if csr firmware failed to load.

[Imre Deak]

On Wed, 2015-10-14 at 11:59 +0530, Animesh Manna wrote:
> 
> On 9/30/2015 7:54 PM, Imre Deak wrote:
> > On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> >> Note that for bxt without dmc, display engine can go to lowest
> >> possible state (dc9), so releasing the rpm reference.
> >>
> >> Cc: Daniel Vetter <daniel.vetter@intel.com>
> >> Cc: Damien Lespiau <damien.lespiau@intel.com>
> >> Cc: Imre Deak <imre.deak@intel.com>
> >> Cc: Sunil Kamath <sunil.kamath@intel.com>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_csr.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> >> index 75a775b..be388da 100644
> >> --- a/drivers/gpu/drm/i915/intel_csr.c
> >> +++ b/drivers/gpu/drm/i915/intel_csr.c
> >> @@ -392,7 +392,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
> >>   
> >>   	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
> >>   out:
> >> -	if (fw_loaded)
> >> +	if (fw_loaded || IS_BROXTON(dev))
> >>   		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> > I don't think this is needed. We disable all display side runtime power
> > management if the firmware is not available. So no need to special case
> > this either imo.
> In display off case for broxton platform we can directly goto dc9 state 
> for which dmc is not needed.
> So, don't want to block runtime power management from display side.

We could also runtime suspend on Skylake without the DMC firmware. We
made the decision that we won't allow that, for no other reason than to
simplify the code. Doing that we don't need to check for the firmware
loaded status for instance to decide whether to enable DC5/6 or not
(which can be racy). So why would we now add the complexity for one
platform when we decidedly removed it from another?

--Imre

> 
> -Animesh
> >
> >>   	else
> >>   		intel_csr_load_status_set(dev_priv, FW_FAILED);
> >
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 03/14] drm/i915/bxt: release rpm reference if csr firmware failed to load.

[Jani Nikula]

On Wed, 14 Oct 2015, Animesh Manna <animesh.manna@intel.com> wrote:
> On 9/30/2015 7:54 PM, Imre Deak wrote:
>> On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
>>> Note that for bxt without dmc, display engine can go to lowest
>>> possible state (dc9), so releasing the rpm reference.
>>>
>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Damien Lespiau <damien.lespiau@intel.com>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Cc: Sunil Kamath <sunil.kamath@intel.com>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_csr.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>>> index 75a775b..be388da 100644
>>> --- a/drivers/gpu/drm/i915/intel_csr.c
>>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>>> @@ -392,7 +392,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>>>   
>>>   	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
>>>   out:
>>> -	if (fw_loaded)
>>> +	if (fw_loaded || IS_BROXTON(dev))
>>>   		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>> I don't think this is needed. We disable all display side runtime power
>> management if the firmware is not available. So no need to special case
>> this either imo.
> In display off case for broxton platform we can directly goto dc9 state 
> for which dmc is not needed.
> So, don't want to block runtime power management from display side.

Your patch doesn't set load status to failed on bxt. There's another
version from Sagar that does [1].

BR,
Jani.


[1] http://mid.gmane.org/1444754985-15734-1-git-send-email-sagar.a.kamble@intel.com


>
> -Animesh
>>
>>>   	else
>>>   		intel_csr_load_status_set(dev_priv, FW_FAILED);
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 04/14] drm/i915/gen9: move assert_csr_loaded into intel_rpm.c

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

Avoids non-static functions since all the callers are in intel_rpm.c.
Only thing we need for that is to move the register definitions into
i915_reg.h.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_csr.c        | 25 -------------------------
 drivers/gpu/drm/i915/intel_drv.h        |  1 -
 drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++++++
 4 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1fa0554..b6bde0c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7392,6 +7392,22 @@ enum skl_disp_power_wells {
 #define  DC_STATE_DEBUG                  0x45520
 #define  DC_STATE_DEBUG_MASK_MEMORY_UP	(1<<1)
 
+/*
+* SKL CSR registers for DC5 and DC6
+*/
+#define CSR_PROGRAM_BASE		0x80000
+#define CSR_SSP_BASE_ADDR_GEN9		0x00002FC0
+#define CSR_HTP_ADDR_SKL		0x00500034
+#define CSR_SSP_BASE			0x8F074
+#define CSR_HTP_SKL			0x8F004
+#define CSR_LAST_WRITE			0x8F034
+#define CSR_LAST_WRITE_VALUE		0xc003b400
+/* MMIO address range for CSR program (0x80000 - 0x82FFF) */
+#define CSR_MAX_FW_SIZE			0x2FFF
+#define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
+#define CSR_MMIO_START_RANGE	0x80000
+#define CSR_MMIO_END_RANGE		0x8FFFF
+
 /* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
  * since on HSW we can't write to it using I915_WRITE. */
 #define D_COMP_HSW			(MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index be388da..9153764 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -45,22 +45,6 @@
 
 MODULE_FIRMWARE(I915_CSR_SKL);
 
-/*
-* SKL CSR registers for DC5 and DC6
-*/
-#define CSR_PROGRAM_BASE		0x80000
-#define CSR_SSP_BASE_ADDR_GEN9		0x00002FC0
-#define CSR_HTP_ADDR_SKL		0x00500034
-#define CSR_SSP_BASE			0x8F074
-#define CSR_HTP_SKL			0x8F004
-#define CSR_LAST_WRITE			0x8F034
-#define CSR_LAST_WRITE_VALUE		0xc003b400
-/* MMIO address range for CSR program (0x80000 - 0x82FFF) */
-#define CSR_MAX_FW_SIZE			0x2FFF
-#define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
-#define CSR_MMIO_START_RANGE	0x80000
-#define CSR_MMIO_END_RANGE		0x8FFFF
-
 struct intel_css_header {
 	/* 0x09 for DMC */
 	uint32_t module_type;
@@ -461,12 +445,3 @@ void intel_csr_ucode_fini(struct drm_device *dev)
 	kfree(dev_priv->csr.dmc_payload);
 }
 
-void assert_csr_loaded(struct drm_i915_private *dev_priv)
-{
-	WARN(intel_csr_load_status_get(dev_priv) != FW_LOADED,
-	     "CSR is not loaded.\n");
-	WARN(!I915_READ(CSR_PROGRAM_BASE),
-				"CSR program storage start is NULL\n");
-	WARN(!I915_READ(CSR_SSP_BASE), "CSR SSP Base Not fine\n");
-	WARN(!I915_READ(CSR_HTP_SKL), "CSR HTP Not fine\n");
-}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9cb7d4e..c178ae8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1157,7 +1157,6 @@ void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
 					enum csr_state state);
 void intel_csr_load_program(struct drm_device *dev);
 void intel_csr_ucode_fini(struct drm_device *dev);
-void assert_csr_loaded(struct drm_i915_private *dev_priv);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 340f386..71832dc 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -455,6 +455,14 @@ static void gen9_set_dc_state_debugmask_memory_up(
 	}
 }
 
+static void assert_csr_loaded(struct drm_i915_private *dev_priv)
+{
+	WARN(!I915_READ(CSR_PROGRAM_BASE),
+				"CSR program storage start is NULL\n");
+	WARN(!I915_READ(CSR_SSP_BASE), "CSR SSP Base Not fine\n");
+	WARN(!I915_READ(CSR_HTP_SKL), "CSR HTP Not fine\n");
+}
+
 static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 05/14] drm/i915/gen9: Remove csr.state, csr_lock and related code.

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

This removes two anti-patterns:
- Locking shouldn't be used to synchronize with async work (of any
  form, whether callbacks, workers or other threads). This is what the
  mutex_lock/unlock seems to have been for in intel_csr_load_program.
  Instead ordering should be ensured with the generic
  wait_for_completion()/complete(). Or more specific functions
  provided by the core kernel like e.g.
  flush_work()/cancel_work_sync() in the case of synchronizing with a
  work item.

- Don't invent own completion like the following code did with the
  (already removed) wait_for(csr_load_status_get()) pattern - it's
  really hard to get these right when you want them to be _really_
  correct (and be fast) in all cases. Furthermore it's easier to read
  code using the well-known primitives than new ones using
  non-standard names.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         |  1 -
 drivers/gpu/drm/i915/i915_drv.c         | 13 ++--------
 drivers/gpu/drm/i915/i915_drv.h         | 10 -------
 drivers/gpu/drm/i915/intel_csr.c        | 46 +--------------------------------
 drivers/gpu/drm/i915/intel_drv.h        |  3 ---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 17 ++----------
 6 files changed, 5 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 48b9792..aa3cdcf 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -839,7 +839,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	spin_lock_init(&dev_priv->mmio_flip_lock);
 	mutex_init(&dev_priv->sb_lock);
 	mutex_init(&dev_priv->modeset_restore_lock);
-	mutex_init(&dev_priv->csr_lock);
 
 	intel_pm_setup(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fa66162..1ddf3a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1013,19 +1013,11 @@ static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
-	enum csr_state state;
 	/* Enabling DC6 is not a hard requirement to enter runtime D3 */
 
 	skl_uninit_cdclk(dev_priv);
 
-	/* TODO: wait for a completion event or
-	 * similar here instead of busy
-	 * waiting using wait_for function.
-	 */
-	wait_for((state = intel_csr_load_status_get(dev_priv)) !=
-			FW_UNINITIALIZED, 1000);
-	if (state == FW_LOADED)
-		skl_enable_dc6(dev_priv);
+	skl_enable_dc6(dev_priv);
 
 	return 0;
 }
@@ -1073,8 +1065,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
-		skl_disable_dc6(dev_priv);
+	skl_disable_dc6(dev_priv);
 
 	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0f3f05..9f01c72 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -736,12 +736,6 @@ struct intel_uncore {
 #define for_each_fw_domain(domain__, dev_priv__, i__) \
 	for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
 
-enum csr_state {
-	FW_UNINITIALIZED = 0,
-	FW_LOADED,
-	FW_FAILED
-};
-
 struct intel_csr {
 	const char *fw_path;
 	uint32_t *dmc_payload;
@@ -749,7 +743,6 @@ struct intel_csr {
 	uint32_t mmio_count;
 	uint32_t mmioaddr[8];
 	uint32_t mmiodata[8];
-	enum csr_state state;
 };
 
 #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
@@ -1714,9 +1707,6 @@ struct drm_i915_private {
 
 	struct intel_csr csr;
 
-	/* Display CSR-related protection */
-	struct mutex csr_lock;
-
 	struct intel_gmbus gmbus[GMBUS_NUM_PINS];
 
 	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9153764..ee2079b 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -184,40 +184,6 @@ static char intel_get_substepping(struct drm_device *dev)
 }
 
 /**
- * intel_csr_load_status_get() - to get firmware loading status.
- * @dev_priv: i915 device.
- *
- * This function helps to get the firmware loading status.
- *
- * Return: Firmware loading status.
- */
-enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv)
-{
-	enum csr_state state;
-
-	mutex_lock(&dev_priv->csr_lock);
-	state = dev_priv->csr.state;
-	mutex_unlock(&dev_priv->csr_lock);
-
-	return state;
-}
-
-/**
- * intel_csr_load_status_set() - help to set firmware loading status.
- * @dev_priv: i915 device.
- * @state: enumeration of firmware loading status.
- *
- * Set the firmware loading status.
- */
-void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
-			enum csr_state state)
-{
-	mutex_lock(&dev_priv->csr_lock);
-	dev_priv->csr.state = state;
-	mutex_unlock(&dev_priv->csr_lock);
-}
-
-/**
  * intel_csr_load_program() - write the firmware from memory to register.
  * @dev: drm device.
  *
@@ -245,7 +211,6 @@ void intel_csr_load_program(struct drm_device *dev)
 	if (I915_READ(CSR_PROGRAM_BASE))
 		return;
 
-	mutex_lock(&dev_priv->csr_lock);
 	fw_size = dev_priv->csr.dmc_fw_size;
 	for (i = 0; i < fw_size; i++)
 		I915_WRITE(CSR_PROGRAM_BASE + i * 4,
@@ -255,9 +220,6 @@ void intel_csr_load_program(struct drm_device *dev)
 		I915_WRITE(dev_priv->csr.mmioaddr[i],
 			dev_priv->csr.mmiodata[i]);
 	}
-
-	dev_priv->csr.state = FW_LOADED;
-	mutex_unlock(&dev_priv->csr_lock);
 }
 
 static void finish_csr_load(const struct firmware *fw, void *context)
@@ -378,8 +340,6 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 out:
 	if (fw_loaded || IS_BROXTON(dev))
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-	else
-		intel_csr_load_status_set(dev_priv, FW_FAILED);
 
 	release_firmware(fw);
 }
@@ -404,7 +364,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
 		csr->fw_path = I915_CSR_SKL;
 	else {
 		DRM_ERROR("Unexpected: no known CSR firmware for platform\n");
-		intel_csr_load_status_set(dev_priv, FW_FAILED);
 		return;
 	}
 
@@ -421,10 +380,8 @@ void intel_csr_ucode_init(struct drm_device *dev)
 				&dev_priv->dev->pdev->dev,
 				GFP_KERNEL, dev_priv,
 				finish_csr_load);
-	if (ret) {
+	if (ret)
 		i915_firmware_load_error_print(csr->fw_path, ret);
-		intel_csr_load_status_set(dev_priv, FW_FAILED);
-	}
 }
 
 /**
@@ -441,7 +398,6 @@ void intel_csr_ucode_fini(struct drm_device *dev)
 	if (!HAS_CSR(dev))
 		return;
 
-	intel_csr_load_status_set(dev_priv, FW_FAILED);
 	kfree(dev_priv->csr.dmc_payload);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c178ae8..a5e4b08 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1152,9 +1152,6 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
 
 /* intel_csr.c */
 void intel_csr_ucode_init(struct drm_device *dev);
-enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv);
-void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
-					enum csr_state state);
 void intel_csr_load_program(struct drm_device *dev);
 void intel_csr_ucode_fini(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 71832dc..3ffebb7 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -661,8 +661,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 	} else {
 		if (enable_requested) {
 			if (IS_SKYLAKE(dev) &&
-				(power_well->data == SKL_DISP_PW_1) &&
-				(intel_csr_load_status_get(dev_priv) == FW_LOADED))
+				(power_well->data == SKL_DISP_PW_1))
 				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
 			else {
 				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
@@ -671,20 +670,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 			}
 
 			if (GEN9_ENABLE_DC5(dev) &&
-				power_well->data == SKL_DISP_PW_2) {
-				enum csr_state state;
-				/* TODO: wait for a completion event or
-				 * similar here instead of busy
-				 * waiting using wait_for function.
-				 */
-				wait_for((state = intel_csr_load_status_get(dev_priv)) !=
-						FW_UNINITIALIZED, 1000);
-				if (state != FW_LOADED)
-					DRM_ERROR("CSR firmware not ready (%d)\n",
-							state);
-				else
+				power_well->data == SKL_DISP_PW_2)
 					gen9_enable_dc5(dev_priv);
-			}
 		}
 	}
 
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 05/14] drm/i915/gen9: Remove csr.state, csr_lock and related code.

[Imre Deak]

On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> From: Daniel Vetter <daniel.vetter@intel.com>
> 
> This removes two anti-patterns:
> - Locking shouldn't be used to synchronize with async work (of any
>   form, whether callbacks, workers or other threads). This is what the
>   mutex_lock/unlock seems to have been for in intel_csr_load_program.
>   Instead ordering should be ensured with the generic
>   wait_for_completion()/complete(). Or more specific functions
>   provided by the core kernel like e.g.
>   flush_work()/cancel_work_sync() in the case of synchronizing with a
>   work item.
> 
> - Don't invent own completion like the following code did with the
>   (already removed) wait_for(csr_load_status_get()) pattern - it's
>   really hard to get these right when you want them to be _really_
>   correct (and be fast) in all cases. Furthermore it's easier to read
>   code using the well-known primitives than new ones using
>   non-standard names.
> 
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c         |  1 -
>  drivers/gpu/drm/i915/i915_drv.c         | 13 ++--------
>  drivers/gpu/drm/i915/i915_drv.h         | 10 -------
>  drivers/gpu/drm/i915/intel_csr.c        | 46 +--------------------------------
>  drivers/gpu/drm/i915/intel_drv.h        |  3 ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 17 ++----------
>  6 files changed, 5 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 48b9792..aa3cdcf 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -839,7 +839,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	spin_lock_init(&dev_priv->mmio_flip_lock);
>  	mutex_init(&dev_priv->sb_lock);
>  	mutex_init(&dev_priv->modeset_restore_lock);
> -	mutex_init(&dev_priv->csr_lock);
>  
>  	intel_pm_setup(dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index fa66162..1ddf3a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1013,19 +1013,11 @@ static int i915_pm_resume(struct device *dev)
>  
>  static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  {
> -	enum csr_state state;
>  	/* Enabling DC6 is not a hard requirement to enter runtime D3 */
>  
>  	skl_uninit_cdclk(dev_priv);
>  
> -	/* TODO: wait for a completion event or
> -	 * similar here instead of busy
> -	 * waiting using wait_for function.
> -	 */
> -	wait_for((state = intel_csr_load_status_get(dev_priv)) !=
> -			FW_UNINITIALIZED, 1000);
> -	if (state == FW_LOADED)
> -		skl_enable_dc6(dev_priv);
> +	skl_enable_dc6(dev_priv);

Here the firmware may not be loaded yet during system suspend. Since
patch 11/14 moves the firmware loading to a work item, we could flush
that work in i915_drm_suspend() and check here if csr->dmc_payload is
set before enabling DC6. (and reorder this patch after patch 11).

>  
>  	return 0;
>  }
> @@ -1073,8 +1065,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> -	if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
> -		skl_disable_dc6(dev_priv);
> +	skl_disable_dc6(dev_priv);

As above we should check here for csr->dmc_payload.

>  	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e0f3f05..9f01c72 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -736,12 +736,6 @@ struct intel_uncore {
>  #define for_each_fw_domain(domain__, dev_priv__, i__) \
>  	for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
>  
> -enum csr_state {
> -	FW_UNINITIALIZED = 0,
> -	FW_LOADED,
> -	FW_FAILED
> -};
> -
>  struct intel_csr {
>  	const char *fw_path;
>  	uint32_t *dmc_payload;
> @@ -749,7 +743,6 @@ struct intel_csr {
>  	uint32_t mmio_count;
>  	uint32_t mmioaddr[8];
>  	uint32_t mmiodata[8];
> -	enum csr_state state;
>  };
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> @@ -1714,9 +1707,6 @@ struct drm_i915_private {
>  
>  	struct intel_csr csr;
>  
> -	/* Display CSR-related protection */
> -	struct mutex csr_lock;
> -
>  	struct intel_gmbus gmbus[GMBUS_NUM_PINS];
>  
>  	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 9153764..ee2079b 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -184,40 +184,6 @@ static char intel_get_substepping(struct drm_device *dev)
>  }
>  
>  /**
> - * intel_csr_load_status_get() - to get firmware loading status.
> - * @dev_priv: i915 device.
> - *
> - * This function helps to get the firmware loading status.
> - *
> - * Return: Firmware loading status.
> - */
> -enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv)
> -{
> -	enum csr_state state;
> -
> -	mutex_lock(&dev_priv->csr_lock);
> -	state = dev_priv->csr.state;
> -	mutex_unlock(&dev_priv->csr_lock);
> -
> -	return state;
> -}
> -
> -/**
> - * intel_csr_load_status_set() - help to set firmware loading status.
> - * @dev_priv: i915 device.
> - * @state: enumeration of firmware loading status.
> - *
> - * Set the firmware loading status.
> - */
> -void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
> -			enum csr_state state)
> -{
> -	mutex_lock(&dev_priv->csr_lock);
> -	dev_priv->csr.state = state;
> -	mutex_unlock(&dev_priv->csr_lock);
> -}
> -
> -/**
>   * intel_csr_load_program() - write the firmware from memory to register.
>   * @dev: drm device.
>   *
> @@ -245,7 +211,6 @@ void intel_csr_load_program(struct drm_device *dev)
>  	if (I915_READ(CSR_PROGRAM_BASE))
>  		return;
>  
> -	mutex_lock(&dev_priv->csr_lock);
>  	fw_size = dev_priv->csr.dmc_fw_size;
>  	for (i = 0; i < fw_size; i++)
>  		I915_WRITE(CSR_PROGRAM_BASE + i * 4,
> @@ -255,9 +220,6 @@ void intel_csr_load_program(struct drm_device *dev)
>  		I915_WRITE(dev_priv->csr.mmioaddr[i],
>  			dev_priv->csr.mmiodata[i]);
>  	}
> -
> -	dev_priv->csr.state = FW_LOADED;
> -	mutex_unlock(&dev_priv->csr_lock);
>  }
>  
>  static void finish_csr_load(const struct firmware *fw, void *context)
> @@ -378,8 +340,6 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  out:
>  	if (fw_loaded || IS_BROXTON(dev))
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> -	else
> -		intel_csr_load_status_set(dev_priv, FW_FAILED);
>  
>  	release_firmware(fw);
>  }
> @@ -404,7 +364,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
>  		csr->fw_path = I915_CSR_SKL;
>  	else {
>  		DRM_ERROR("Unexpected: no known CSR firmware for platform\n");
> -		intel_csr_load_status_set(dev_priv, FW_FAILED);
>  		return;
>  	}
>  
> @@ -421,10 +380,8 @@ void intel_csr_ucode_init(struct drm_device *dev)
>  				&dev_priv->dev->pdev->dev,
>  				GFP_KERNEL, dev_priv,
>  				finish_csr_load);
> -	if (ret) {
> +	if (ret)
>  		i915_firmware_load_error_print(csr->fw_path, ret);
> -		intel_csr_load_status_set(dev_priv, FW_FAILED);
> -	}
>  }
>  
>  /**
> @@ -441,7 +398,6 @@ void intel_csr_ucode_fini(struct drm_device *dev)
>  	if (!HAS_CSR(dev))
>  		return;
>  
> -	intel_csr_load_status_set(dev_priv, FW_FAILED);
>  	kfree(dev_priv->csr.dmc_payload);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c178ae8..a5e4b08 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1152,9 +1152,6 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
>  
>  /* intel_csr.c */
>  void intel_csr_ucode_init(struct drm_device *dev);
> -enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv);
> -void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
> -					enum csr_state state);
>  void intel_csr_load_program(struct drm_device *dev);
>  void intel_csr_ucode_fini(struct drm_device *dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 71832dc..3ffebb7 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -661,8 +661,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  	} else {
>  		if (enable_requested) {
>  			if (IS_SKYLAKE(dev) &&
> -				(power_well->data == SKL_DISP_PW_1) &&
> -				(intel_csr_load_status_get(dev_priv) == FW_LOADED))
> +				(power_well->data == SKL_DISP_PW_1))
>  				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
>  			else {
>  				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
> @@ -671,20 +670,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  			}
>  
>  			if (GEN9_ENABLE_DC5(dev) &&
> -				power_well->data == SKL_DISP_PW_2) {
> -				enum csr_state state;
> -				/* TODO: wait for a completion event or
> -				 * similar here instead of busy
> -				 * waiting using wait_for function.
> -				 */
> -				wait_for((state = intel_csr_load_status_get(dev_priv)) !=
> -						FW_UNINITIALIZED, 1000);
> -				if (state != FW_LOADED)
> -					DRM_ERROR("CSR firmware not ready (%d)\n",
> -							state);
> -				else
> +				power_well->data == SKL_DISP_PW_2)
>  					gen9_enable_dc5(dev_priv);
> -			}
>  		}
>  	}
>  


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 06/14] drm/i915/gen9: Align line continuations in intel_csr.c.

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

Standard is to align continuations of parameter lists and if
conditions to the opening ( in i915 and drm code.

Apply this across the entire file since it was sticking out a bit too
much.

Also align register definitions while at it.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_csr.c | 42 ++++++++++++++++++++--------------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6bde0c..30147f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7405,7 +7405,7 @@ enum skl_disp_power_wells {
 /* MMIO address range for CSR program (0x80000 - 0x82FFF) */
 #define CSR_MAX_FW_SIZE			0x2FFF
 #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
-#define CSR_MMIO_START_RANGE	0x80000
+#define CSR_MMIO_START_RANGE		0x80000
 #define CSR_MMIO_END_RANGE		0x8FFFF
 
 /* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ee2079b..00e9fbf 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -160,15 +160,15 @@ struct stepping_info {
 };
 
 static const struct stepping_info skl_stepping_info[] = {
-		{'A', '0'}, {'B', '0'}, {'C', '0'},
-		{'D', '0'}, {'E', '0'}, {'F', '0'},
-		{'G', '0'}, {'H', '0'}, {'I', '0'}
+	{'A', '0'}, {'B', '0'}, {'C', '0'},
+	{'D', '0'}, {'E', '0'}, {'F', '0'},
+	{'G', '0'}, {'H', '0'}, {'I', '0'}
 };
 
 static char intel_get_stepping(struct drm_device *dev)
 {
 	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
-			ARRAY_SIZE(skl_stepping_info)))
+				ARRAY_SIZE(skl_stepping_info)))
 		return skl_stepping_info[dev->pdev->revision].stepping;
 	else
 		return -ENODATA;
@@ -177,7 +177,7 @@ static char intel_get_stepping(struct drm_device *dev)
 static char intel_get_substepping(struct drm_device *dev)
 {
 	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
-			ARRAY_SIZE(skl_stepping_info)))
+				ARRAY_SIZE(skl_stepping_info)))
 		return skl_stepping_info[dev->pdev->revision].substepping;
 	else
 		return -ENODATA;
@@ -214,11 +214,11 @@ void intel_csr_load_program(struct drm_device *dev)
 	fw_size = dev_priv->csr.dmc_fw_size;
 	for (i = 0; i < fw_size; i++)
 		I915_WRITE(CSR_PROGRAM_BASE + i * 4,
-			payload[i]);
+			   payload[i]);
 
 	for (i = 0; i < dev_priv->csr.mmio_count; i++) {
 		I915_WRITE(dev_priv->csr.mmioaddr[i],
-			dev_priv->csr.mmiodata[i]);
+			   dev_priv->csr.mmiodata[i]);
 	}
 }
 
@@ -250,20 +250,20 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	/* Extract CSS Header information*/
 	css_header = (struct intel_css_header *)fw->data;
 	if (sizeof(struct intel_css_header) !=
-		(css_header->header_len * 4)) {
+	    (css_header->header_len * 4)) {
 		DRM_ERROR("Firmware has wrong CSS header length %u bytes\n",
-			(css_header->header_len * 4));
+			  (css_header->header_len * 4));
 		goto out;
 	}
 	readcount += sizeof(struct intel_css_header);
 
 	/* Extract Package Header information*/
 	package_header = (struct intel_package_header *)
-					&fw->data[readcount];
+		&fw->data[readcount];
 	if (sizeof(struct intel_package_header) !=
-		(package_header->header_len * 4)) {
+	    (package_header->header_len * 4)) {
 		DRM_ERROR("Firmware has wrong package header length %u bytes\n",
-			(package_header->header_len * 4));
+			  (package_header->header_len * 4));
 		goto out;
 	}
 	readcount += sizeof(struct intel_package_header);
@@ -271,7 +271,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	/* Search for dmc_offset to find firware binary. */
 	for (i = 0; i < package_header->num_entries; i++) {
 		if (package_header->fw_info[i].substepping == '*' &&
-			stepping == package_header->fw_info[i].stepping) {
+		    stepping == package_header->fw_info[i].stepping) {
 			dmc_offset = package_header->fw_info[i].offset;
 			break;
 		} else if (stepping == package_header->fw_info[i].stepping &&
@@ -279,7 +279,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 			dmc_offset = package_header->fw_info[i].offset;
 			break;
 		} else if (package_header->fw_info[i].stepping == '*' &&
-			package_header->fw_info[i].substepping == '*')
+			   package_header->fw_info[i].substepping == '*')
 			dmc_offset = package_header->fw_info[i].offset;
 	}
 	if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
@@ -292,7 +292,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
 	if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
 		DRM_ERROR("Firmware has wrong dmc header length %u bytes\n",
-				(dmc_header->header_len));
+			  (dmc_header->header_len));
 		goto out;
 	}
 	readcount += sizeof(struct intel_dmc_header);
@@ -300,15 +300,15 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	/* Cache the dmc header info. */
 	if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
 		DRM_ERROR("Firmware has wrong mmio count %u\n",
-						dmc_header->mmio_count);
+			  dmc_header->mmio_count);
 		goto out;
 	}
 	csr->mmio_count = dmc_header->mmio_count;
 	for (i = 0; i < dmc_header->mmio_count; i++) {
 		if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE &&
-			dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
+		    dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
 			DRM_ERROR(" Firmware has wrong mmio address 0x%x\n",
-						dmc_header->mmioaddr[i]);
+				  dmc_header->mmioaddr[i]);
 			goto out;
 		}
 		csr->mmioaddr[i] = dmc_header->mmioaddr[i];
@@ -377,9 +377,9 @@ void intel_csr_ucode_init(struct drm_device *dev)
 
 	/* CSR supported for platform, load firmware */
 	ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path,
-				&dev_priv->dev->pdev->dev,
-				GFP_KERNEL, dev_priv,
-				finish_csr_load);
+				      &dev_priv->dev->pdev->dev,
+				      GFP_KERNEL, dev_priv,
+				      finish_csr_load);
 	if (ret)
 		i915_firmware_load_error_print(csr->fw_path, ret);
 }
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

If we really want to we can be more verbose here, but we really don't
need an entire function for this.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 20 --------------------
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_csr.c |  8 +++-----
 3 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ddf3a9..3cdd319 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -537,26 +537,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	return true;
 }
 
-void i915_firmware_load_error_print(const char *fw_path, int err)
-{
-	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
-
-	/*
-	 * If the reason is not known assume -ENOENT since that's the most
-	 * usual failure mode.
-	 */
-	if (!err)
-		err = -ENOENT;
-
-	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
-		return;
-
-	DRM_ERROR(
-	  "The driver is built-in, so to load the firmware you need to\n"
-	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
-	  "in your initrd/initramfs image.\n");
-}
-
 static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9f01c72..6241b49 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2654,7 +2654,6 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
 extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
 extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
-void i915_firmware_load_error_print(const char *fw_path, int err);
 
 /* intel_hotplug.c */
 void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 00e9fbf..9d4b37b 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -237,10 +237,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	uint32_t *dmc_payload;
 	bool fw_loaded = false;
 
-	if (!fw) {
-		i915_firmware_load_error_print(csr->fw_path, 0);
+	if (!fw)
 		goto out;
-	}
 
 	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
 		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
@@ -340,6 +338,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 out:
 	if (fw_loaded || IS_BROXTON(dev))
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+	else
+		DRM_ERROR("Failed to load DMC firmware, disabling rpm\n");
 
 	release_firmware(fw);
 }
@@ -380,8 +380,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
 				      &dev_priv->dev->pdev->dev,
 				      GFP_KERNEL, dev_priv,
 				      finish_csr_load);
-	if (ret)
-		i915_firmware_load_error_print(csr->fw_path, ret);
 }
 
 /**
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

[Imre Deak]

On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> From: Daniel Vetter <daniel.vetter@intel.com>
> 
> If we really want to we can be more verbose here, but we really don't
> need an entire function for this.
> 
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c  | 20 --------------------
>  drivers/gpu/drm/i915/i915_drv.h  |  1 -
>  drivers/gpu/drm/i915/intel_csr.c |  8 +++-----
>  3 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ddf3a9..3cdd319 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -537,26 +537,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	return true;
>  }
>  
> -void i915_firmware_load_error_print(const char *fw_path, int err)
> -{
> -	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> -
> -	/*
> -	 * If the reason is not known assume -ENOENT since that's the most
> -	 * usual failure mode.
> -	 */
> -	if (!err)
> -		err = -ENOENT;
> -
> -	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
> -		return;
> -
> -	DRM_ERROR(
> -	  "The driver is built-in, so to load the firmware you need to\n"
> -	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> -	  "in your initrd/initramfs image.\n");
> -}
> -

The point here was to clarify the reason why the loading failed, since
that caused quite a confusion. It was a separate function since the same
could've been called from the GuC loader too. I think the error message
would be still useful.

>  static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9f01c72..6241b49 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2654,7 +2654,6 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
>  extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
>  extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
>  int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
> -void i915_firmware_load_error_print(const char *fw_path, int err);
>  
>  /* intel_hotplug.c */
>  void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 00e9fbf..9d4b37b 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -237,10 +237,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  	uint32_t *dmc_payload;
>  	bool fw_loaded = false;
>  
> -	if (!fw) {
> -		i915_firmware_load_error_print(csr->fw_path, 0);
> +	if (!fw)
>  		goto out;
> -	}
>  
>  	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
>  		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
> @@ -340,6 +338,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  out:
>  	if (fw_loaded || IS_BROXTON(dev))
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> +	else
> +		DRM_ERROR("Failed to load DMC firmware, disabling rpm\n");
>  
>  	release_firmware(fw);
>  }
> @@ -380,8 +380,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
>  				      &dev_priv->dev->pdev->dev,
>  				      GFP_KERNEL, dev_priv,
>  				      finish_csr_load);
> -	if (ret)
> -		i915_firmware_load_error_print(csr->fw_path, ret);
>  }
>  
>  /**


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

[Marc Herbert]

On 30/09/15 07:28, Imre Deak wrote:
> On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
>>
>> -void i915_firmware_load_error_print(const char *fw_path, int err)
>> -{
>> -	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
>> -
>> -	/*
>> -	 * If the reason is not known assume -ENOENT since that's the most
>> -	 * usual failure mode.
>> -	 */
>> -	if (!err)
>> -		err = -ENOENT;
>> -
>> -	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
>> -		return;
>> -
>> -	DRM_ERROR(
>> -	  "The driver is built-in, so to load the firmware you need to\n"
>> -	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
>> -	  "in your initrd/initramfs image.\n");
>> -}
>> -
>
> The point here was to clarify the reason why the loading failed, since
> that caused quite a confusion. It was a separate function since the same
> could've been called from the GuC loader too. I think the error message
> would be still useful.

Agreed 100%. The code of this function was a bit confusing, however this 
error message has proved very useful "on the field" many times already. 
Please preserve the message.


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

[Daniel Vetter]

On Tue, Oct 06, 2015 at 12:38:00PM -0700, Marc Herbert wrote:
> On 30/09/15 07:28, Imre Deak wrote:
> >On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> >>
> >>-void i915_firmware_load_error_print(const char *fw_path, int err)
> >>-{
> >>-	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> >>-
> >>-	/*
> >>-	 * If the reason is not known assume -ENOENT since that's the most
> >>-	 * usual failure mode.
> >>-	 */
> >>-	if (!err)
> >>-		err = -ENOENT;
> >>-
> >>-	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
> >>-		return;
> >>-
> >>-	DRM_ERROR(
> >>-	  "The driver is built-in, so to load the firmware you need to\n"
> >>-	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> >>-	  "in your initrd/initramfs image.\n");
> >>-}
> >>-
> >
> >The point here was to clarify the reason why the loading failed, since
> >that caused quite a confusion. It was a separate function since the same
> >could've been called from the GuC loader too. I think the error message
> >would be still useful.
> 
> Agreed 100%. The code of this function was a bit confusing, however this
> error message has proved very useful "on the field" many times already.
> Please preserve the message.

I dunno really, this is pretty much known by all the other gpu drivers. If
you load things before the firmware is there it'll fail. Only difference
is that with i915 dmc is the first time we require firmware images for
some features.

We should document this in the release notes for the skl/bxt+ stacks, but
this really isn't all that useful in the kernel imo. It's really just how
fw loading works (with drm drivers at least), and at least distros do have
that operational knowledge since years.

Also only printing something for the built-in case seems wrong, since you
can equally screw things up when it's the driver is module. What we should
have instead is just a DRM_INFO or similar that we're disabling a few pm
features because the firmware isn't there.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 08/14] drm/i915/gen9: extract parse_csr_fw

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

The loader function will get a bit more complicated soon, extract the
parsing code to make the control flow clearer. While doing that just
use dev_priv->csr.dmc_payload as the indicator for whether it all
suceeded or not.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 48 +++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9d4b37b..9971794 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -222,9 +222,9 @@ void intel_csr_load_program(struct drm_device *dev)
 	}
 }
 
-static void finish_csr_load(const struct firmware *fw, void *context)
+static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
+			      const struct firmware *fw)
 {
-	struct drm_i915_private *dev_priv = context;
 	struct drm_device *dev = dev_priv->dev;
 	struct intel_css_header *css_header;
 	struct intel_package_header *package_header;
@@ -235,14 +235,13 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
 	uint32_t i;
 	uint32_t *dmc_payload;
-	bool fw_loaded = false;
 
 	if (!fw)
-		goto out;
+		return NULL;
 
 	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
 		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
-		goto out;
+		return NULL;
 	}
 
 	/* Extract CSS Header information*/
@@ -251,7 +250,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	    (css_header->header_len * 4)) {
 		DRM_ERROR("Firmware has wrong CSS header length %u bytes\n",
 			  (css_header->header_len * 4));
-		goto out;
+		return NULL;
 	}
 	readcount += sizeof(struct intel_css_header);
 
@@ -262,7 +261,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	    (package_header->header_len * 4)) {
 		DRM_ERROR("Firmware has wrong package header length %u bytes\n",
 			  (package_header->header_len * 4));
-		goto out;
+		return NULL;
 	}
 	readcount += sizeof(struct intel_package_header);
 
@@ -282,7 +281,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	}
 	if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
 		DRM_ERROR("Firmware not supported for %c stepping\n", stepping);
-		goto out;
+		return NULL;
 	}
 	readcount += dmc_offset;
 
@@ -291,7 +290,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
 		DRM_ERROR("Firmware has wrong dmc header length %u bytes\n",
 			  (dmc_header->header_len));
-		goto out;
+		return NULL;
 	}
 	readcount += sizeof(struct intel_dmc_header);
 
@@ -299,7 +298,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
 		DRM_ERROR("Firmware has wrong mmio count %u\n",
 			  dmc_header->mmio_count);
-		goto out;
+		return NULL;
 	}
 	csr->mmio_count = dmc_header->mmio_count;
 	for (i = 0; i < dmc_header->mmio_count; i++) {
@@ -307,7 +306,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 		    dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
 			DRM_ERROR(" Firmware has wrong mmio address 0x%x\n",
 				  dmc_header->mmioaddr[i]);
-			goto out;
+			return NULL;
 		}
 		csr->mmioaddr[i] = dmc_header->mmioaddr[i];
 		csr->mmiodata[i] = dmc_header->mmiodata[i];
@@ -317,26 +316,39 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 	nbytes = dmc_header->fw_size * 4;
 	if (nbytes > CSR_MAX_FW_SIZE) {
 		DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes);
-		goto out;
+		return NULL;
 	}
 	csr->dmc_fw_size = dmc_header->fw_size;
 
-	csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL);
-	if (!csr->dmc_payload) {
+	dmc_payload = kmalloc(nbytes, GFP_KERNEL);
+	if (!dmc_payload) {
 		DRM_ERROR("Memory allocation failed for dmc payload\n");
-		goto out;
+		return NULL;
 	}
 
-	dmc_payload = csr->dmc_payload;
 	memcpy(dmc_payload, &fw->data[readcount], nbytes);
 
+	return dmc_payload;
+}
+
+static void finish_csr_load(const struct firmware *fw, void *context)
+{
+	struct drm_i915_private *dev_priv = context;
+	struct drm_device *dev = dev_priv->dev;
+
+	if (!fw)
+		goto out;
+
+	dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
+	if (!dev_priv->csr.dmc_payload)
+		goto out;
+
 	/* load csr program during system boot, as needed for DC states */
 	intel_csr_load_program(dev);
-	fw_loaded = true;
 
 	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
 out:
-	if (fw_loaded || IS_BROXTON(dev))
+	if (dev_priv->csr.dmc_payload || IS_BROXTON(dev))
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 	else
 		DRM_ERROR("Failed to load DMC firmware, disabling rpm\n");
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 09/14] drm/i915/gen9: Don't try to load garbage dmc firmware on resume

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

We need to make sure we don't put garbage into the hw if dmc firmware
loading failed mid-thru.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9971794..9ed513c 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -208,7 +208,7 @@ void intel_csr_load_program(struct drm_device *dev)
 	 * This condition will help to check if csr address space is reset/
 	 * not loaded.
 	 */
-	if (I915_READ(CSR_PROGRAM_BASE))
+	if ((!dev_priv->csr.dmc_payload) || I915_READ(CSR_PROGRAM_BASE))
 		return;
 
 	fw_size = dev_priv->csr.dmc_fw_size;
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 10/14] drm/i915/gen9: Use dev_priv in csr functions

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

As all csr firmware related opertion are not using any
any data structures of drm framework level, so better to
use dev_priv instead of dev. it's a new style! :)

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c  |  6 +++---
 drivers/gpu/drm/i915/i915_drv.c  |  4 +---
 drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h |  6 +++---
 4 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index aa3cdcf..d4a7725 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1032,7 +1032,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	i915_audio_component_init(dev_priv);
 
 	/* Load CSR Firmware for SKL */
-	intel_csr_ucode_init(dev);
+	intel_csr_ucode_init(dev_priv);
 
 	return 0;
 
@@ -1060,7 +1060,7 @@ out_mtrrfree:
 out_gtt:
 	i915_global_gtt_cleanup(dev);
 out_freecsr:
-	intel_csr_ucode_fini(dev);
+	intel_csr_ucode_fini(dev_priv);
 	intel_uncore_fini(dev);
 	pci_iounmap(dev->pdev, dev_priv->regs);
 put_bridge:
@@ -1142,7 +1142,7 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_fbc_cleanup_cfb(dev_priv);
 	i915_gem_cleanup_stolen(dev);
 
-	intel_csr_ucode_fini(dev);
+	intel_csr_ucode_fini(dev_priv);
 
 	intel_teardown_gmbus(dev);
 	intel_teardown_mchbar(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3cdd319..daaf72d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1043,12 +1043,10 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
 
 static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = dev_priv->dev;
-
 	skl_disable_dc6(dev_priv);
 
 	skl_init_cdclk(dev_priv);
-	intel_csr_load_program(dev);
+	intel_csr_load_program(dev_priv);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9ed513c..b369ea4 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -185,19 +185,18 @@ static char intel_get_substepping(struct drm_device *dev)
 
 /**
  * intel_csr_load_program() - write the firmware from memory to register.
- * @dev: drm device.
+ * @dev_priv: i915 drm device.
  *
  * CSR firmware is read from a .bin file and kept in internal memory one time.
  * Everytime display comes back from low power state this function is called to
  * copy the firmware from internal memory to registers.
  */
-void intel_csr_load_program(struct drm_device *dev)
+void intel_csr_load_program(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 *payload = dev_priv->csr.dmc_payload;
 	uint32_t i, fw_size;
 
-	if (!IS_GEN9(dev)) {
+	if (!IS_GEN9(dev_priv)) {
 		DRM_ERROR("No CSR support available for this platform\n");
 		return;
 	}
@@ -334,7 +333,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 static void finish_csr_load(const struct firmware *fw, void *context)
 {
 	struct drm_i915_private *dev_priv = context;
-	struct drm_device *dev = dev_priv->dev;
 
 	if (!fw)
 		goto out;
@@ -344,11 +342,11 @@ static void finish_csr_load(const struct firmware *fw, void *context)
 		goto out;
 
 	/* load csr program during system boot, as needed for DC states */
-	intel_csr_load_program(dev);
+	intel_csr_load_program(dev_priv);
 
 	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
 out:
-	if (dev_priv->csr.dmc_payload || IS_BROXTON(dev))
+	if (dev_priv->csr.dmc_payload || IS_BROXTON(dev_priv))
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 	else
 		DRM_ERROR("Failed to load DMC firmware, disabling rpm\n");
@@ -358,21 +356,20 @@ out:
 
 /**
  * intel_csr_ucode_init() - initialize the firmware loading.
- * @dev: drm device.
+ * @dev_priv: i915 drm device.
  *
  * This function is called at the time of loading the display driver to read
  * firmware from a .bin file and copied into a internal memory.
  */
-void intel_csr_ucode_init(struct drm_device *dev)
+void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_csr *csr = &dev_priv->csr;
 	int ret;
 
-	if (!HAS_CSR(dev))
+	if (!HAS_CSR(dev_priv))
 		return;
 
-	if (IS_SKYLAKE(dev))
+	if (IS_SKYLAKE(dev_priv))
 		csr->fw_path = I915_CSR_SKL;
 	else {
 		DRM_ERROR("Unexpected: no known CSR firmware for platform\n");
@@ -396,16 +393,14 @@ void intel_csr_ucode_init(struct drm_device *dev)
 
 /**
  * intel_csr_ucode_fini() - unload the CSR firmware.
- * @dev: drm device.
+ * @dev_priv: i915 drm device.
  *
  * Firmmware unloading includes freeing the internal momory and reset the
  * firmware loading status.
  */
-void intel_csr_ucode_fini(struct drm_device *dev)
+void intel_csr_ucode_fini(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!HAS_CSR(dev))
+	if (!HAS_CSR(dev_priv))
 		return;
 
 	kfree(dev_priv->csr.dmc_payload);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a5e4b08..62ed7dc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1151,9 +1151,9 @@ u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
 u32 skl_plane_ctl_rotation(unsigned int rotation);
 
 /* intel_csr.c */
-void intel_csr_ucode_init(struct drm_device *dev);
-void intel_csr_load_program(struct drm_device *dev);
-void intel_csr_ucode_fini(struct drm_device *dev);
+void intel_csr_ucode_init(struct drm_i915_private *);
+void intel_csr_load_program(struct drm_i915_private *);
+void intel_csr_ucode_fini(struct drm_i915_private *);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 11/14] drm/i915: Use request_firmware and our own async work

[Animesh Manna]

From: Daniel Vetter <daniel.vetter@intel.com>

Two benefits:
- We can use FW_LOADER_USERSPACE_FALLBACK.
- We can use flush_work to synchronize with the oustanding worker,
  which is a notch more obvious what it does than having a special
  completion.

The next patch will properly synchronize against the async loader in
the resume and unload code.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_csr.c | 19 +++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6241b49..5c34a9f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -737,6 +737,7 @@ struct intel_uncore {
 	for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
 
 struct intel_csr {
+	struct work_struct work;
 	const char *fw_path;
 	uint32_t *dmc_payload;
 	uint32_t dmc_fw_size;
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index b369ea4..f22b368 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -330,10 +330,16 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	return dmc_payload;
 }
 
-static void finish_csr_load(const struct firmware *fw, void *context)
+static void csr_load_work_fn(struct work_struct *work)
 {
-	struct drm_i915_private *dev_priv = context;
+	struct drm_i915_private *dev_priv;
+	const struct firmware *fw;
+	int ret;
+
+	dev_priv = container_of(work, typeof(*dev_priv), csr.work);
 
+	ret = request_firmware(&fw, dev_priv->csr.fw_path,
+			       &dev_priv->dev->pdev->dev);
 	if (!fw)
 		goto out;
 
@@ -364,7 +370,8 @@ out:
 void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
 {
 	struct intel_csr *csr = &dev_priv->csr;
-	int ret;
+
+	INIT_WORK(&dev_priv->csr.work, csr_load_work_fn);
 
 	if (!HAS_CSR(dev_priv))
 		return;
@@ -384,11 +391,7 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
 	 */
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
-	/* CSR supported for platform, load firmware */
-	ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path,
-				      &dev_priv->dev->pdev->dev,
-				      GFP_KERNEL, dev_priv,
-				      finish_csr_load);
+	schedule_work(&dev_priv->csr.work);
 }
 
 /**
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 12/14] drm/i915/gen9: Use flush_work to synchronize with dmc loader

[Animesh Manna]

During driver unload to ensure we dont have any pending task,
flush_work added to complete firmware loading task.

v1: Initial version.

v2: As per review comments from Daniel,
Removed flush_work from skl_set_power_well. As we have taken
power well refernece and rpm count during firmware loading
by using display_power_domain_get/put - this will always
ensure rpm will be blocked if firmware is not loaded.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 2 --
 drivers/gpu/drm/i915/intel_csr.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index daaf72d..e16f8a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -993,8 +993,6 @@ static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
-	/* Enabling DC6 is not a hard requirement to enter runtime D3 */
-
 	skl_uninit_cdclk(dev_priv);
 
 	skl_enable_dc6(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index f22b368..3ddf598 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -406,6 +406,8 @@ void intel_csr_ucode_fini(struct drm_i915_private *dev_priv)
 	if (!HAS_CSR(dev_priv))
 		return;
 
+	flush_work(&dev_priv->csr.work);
+
 	kfree(dev_priv->csr.dmc_payload);
 }
 
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 13/14] drm/i915/skl: Removed assert for csr-fw-loading check during disabling dc6

[Animesh Manna]

As during disabling dc6 no need to check for csr firmware
loading status, so removed the assert call (Requested by Damien).

Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3ffebb7..7089411 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,6 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
 	if (dev_priv->power_domains.initializing)
 		return;
 
-	assert_csr_loaded(dev_priv);
 	WARN(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6),
 		"DC6 already programmed to be disabled.\n");
 }
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DMC_REDESIGN_V2 14/14] drm/i915/gen9: Corrected the sanity check of mmio address range for csr.

[Animesh Manna]

Condition check for out of boundary for csr address space is corrected
(Thanks to David Binderman for suggestion).

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 3ddf598..477e769 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -301,7 +301,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	}
 	csr->mmio_count = dmc_header->mmio_count;
 	for (i = 0; i < dmc_header->mmio_count; i++) {
-		if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE &&
+		if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
 		    dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
 			DRM_ERROR(" Firmware has wrong mmio address 0x%x\n",
 				  dmc_header->mmioaddr[i]);
-- 
2.0.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DMC_REDESIGN_V2 00/14] Redesign dmc firmware loading.

[Animesh Manna]

On 8/26/2015 4:58 PM, Animesh Manna wrote:
> This patch series has the changes done to redesign the dmc firmware
> loading flow. This is continuation of the below patch series after
> addressing review comments from Daniel.
>
> v1: http://lists.freedesktop.org/archives/intel-gfx/2015-August/072921.html
>
> v2:
> - After rebasing based on below patch series.
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/072870.html
Correction! - please consider the below link instead of above link.
http://lists.freedesktop.org/archives/intel-gfx/2015-August/074336.html

-Animesh
> - Two more patches added in this patch series from older patch series.
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071163.html
>
> Animesh Manna (5):
>    drm/i915/gen9: csr_init after runtime pm enable
>    drm/i915/bxt: release rpm reference if csr firmware failed to load.
>    drm/i915/gen9: Use flush_work to synchronize with dmc loader
>    drm/i915/skl: Removed assert for csr-fw-loading check during disabling
>      dc6
>    drm/i915/gen9: Corrected the sanity check of mmio address range for
>      csr.
>
> Daniel Vetter (9):
>    drm/i915: use correct power domain for csr loading
>    drm/i915/gen9: move assert_csr_loaded into intel_rpm.c
>    drm/i915/gen9: Remove csr.state, csr_lock and related code.
>    drm/i915/gen9: Align line continuations in intel_csr.c.
>    drm/i915/gen9: Simplify csr loading failure printing.
>    drm/i915/gen9: extract parse_csr_fw
>    drm/i915/gen9: Don't try to load garbage dmc firmware on resume
>    drm/i915/gen9: Use dev_priv in csr functions
>    drm/i915: Use request_firmware and our own async work
>
>   drivers/gpu/drm/i915/i915_dma.c         |  11 +-
>   drivers/gpu/drm/i915/i915_drv.c         |  39 +-----
>   drivers/gpu/drm/i915/i915_drv.h         |  12 +-
>   drivers/gpu/drm/i915/i915_reg.h         |  16 +++
>   drivers/gpu/drm/i915/intel_csr.c        | 211 ++++++++++++--------------------
>   drivers/gpu/drm/i915/intel_drv.h        |  10 +-
>   drivers/gpu/drm/i915/intel_runtime_pm.c |  26 ++--
>   7 files changed, 114 insertions(+), 211 deletions(-)
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx