All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  7:59 [PATCH] drm/i915: Release power well if load DMC failed Lee, Shawn C
@ 2018-11-21  7:53 ` Jani Nikula
  2018-11-21  8:15   ` Lee, Shawn C
  2018-11-21  8:01 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-11-21 13:46 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2018-11-21  7:53 UTC (permalink / raw)
  To: Lee, Shawn C, intel-gfx; +Cc: Cooper Chiou, Lee, Rodrigo Vivi

On Tue, 20 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> Driver obtain power well at intel_csr_ucode_init().
> And release it after load DMC firmware successful.

Correct.

> An issue happened when DMC was not found or failed
> to load. Power well would not be released and just
> output some error messages. Driver have to release
> power well properly to keep put/get balance.

No. We intentionally do not release it until dmc firmware load succeeds.

See the comment in intel_csr_ucode_init(), as well as this in the branch
where dmc load fails:

		dev_notice(dev_priv->drm.dev,
			   "Failed to load DMC firmware %s."
			   " Disabling runtime power management.\n",
			   csr->fw_path);

We don't support runtime pm without dmc on platforms with dmc.

BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jose Roberto de Souza <jose.souza@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index a516697bf57d..8d04d7b6f00a 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>  	if (dev_priv->csr.dmc_payload) {
>  		intel_csr_load_program(dev_priv);
>  
> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> -
>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>  			 dev_priv->csr.fw_path,
>  			 CSR_VERSION_MAJOR(csr->version),
> @@ -440,6 +438,7 @@ static void csr_load_work_fn(struct work_struct *work)
>  			   INTEL_UC_FIRMWARE_URL);
>  	}
>  
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>  	release_firmware(fw);
>  }

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Release power well if load DMC failed
@ 2018-11-21  7:59 Lee, Shawn C
  2018-11-21  7:53 ` Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Lee, Shawn C @ 2018-11-21  7:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Lee, Jani Nikula, Rodrigo Vivi

Driver obtain power well at intel_csr_ucode_init().
And release it after load DMC firmware successful.

An issue happened when DMC was not found or failed
to load. Power well would not be released and just
output some error messages. Driver have to release
power well properly to keep put/get balance.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jose Roberto de Souza <jose.souza@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>

Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index a516697bf57d..8d04d7b6f00a 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
 	if (dev_priv->csr.dmc_payload) {
 		intel_csr_load_program(dev_priv);
 
-		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-
 		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
 			 dev_priv->csr.fw_path,
 			 CSR_VERSION_MAJOR(csr->version),
@@ -440,6 +438,7 @@ static void csr_load_work_fn(struct work_struct *work)
 			   INTEL_UC_FIRMWARE_URL);
 	}
 
+	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 	release_firmware(fw);
 }
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Release power well if load DMC failed
  2018-11-21  7:59 [PATCH] drm/i915: Release power well if load DMC failed Lee, Shawn C
  2018-11-21  7:53 ` Jani Nikula
@ 2018-11-21  8:01 ` Patchwork
  2018-11-21 13:46 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-11-21  8:01 UTC (permalink / raw)
  To: Lee, Shawn C; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Release power well if load DMC failed
URL   : https://patchwork.freedesktop.org/series/52805/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5175 -> Patchwork_10875 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52805/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10875 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_switch@basic-default:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@i915_selftest@live_hangcheck:
      fi-bwr-2160:        PASS -> DMESG-FAIL (fdo#108735)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735


== Participating hosts (52 -> 45) ==

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-ivb-3520m 


== Build changes ==

    * Linux: CI_DRM_5175 -> Patchwork_10875

  CI_DRM_5175: eccbba7017aafd70e09bb105c8a6b85572a93eb8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4723: 53bb24ad410b53cdd96f15ced8fd5921c8ab0eac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10875: 285be726370fd7560644f653b3428ab4f66af9fb @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

285be726370f drm/i915: Release power well if load DMC failed

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10875/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  7:53 ` Jani Nikula
@ 2018-11-21  8:15   ` Lee, Shawn C
  2018-11-21  9:05     ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Lee, Shawn C @ 2018-11-21  8:15 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx; +Cc: Chiou, Cooper, Vivi, Rodrigo


On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>> Driver obtain power well at intel_csr_ucode_init().
>> And release it after load DMC firmware successful.
>
>Correct.
>
>> An issue happened when DMC was not found or failed to load. Power well 
>> would not be released and just output some error messages. Driver have 
>> to release power well properly to keep put/get balance.
>
>No. We intentionally do not release it until dmc firmware load succeeds.

If load DMC failed, we found DP phy was always on even without external display connected.
So it looks like an expected behavior, right?

>
>See the comment in intel_csr_ucode_init(), as well as this in the branch where dmc load fails:
>
>		dev_notice(dev_priv->drm.dev,
>			   "Failed to load DMC firmware %s."
>			   " Disabling runtime power management.\n",
>			   csr->fw_path);
>
>We don't support runtime pm without dmc on platforms with dmc.
>
>BR,
>Jani.
>
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>
>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c 
>> b/drivers/gpu/drm/i915/intel_csr.c
>> index a516697bf57d..8d04d7b6f00a 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>>  	if (dev_priv->csr.dmc_payload) {
>>  		intel_csr_load_program(dev_priv);
>>  
>> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>> -
>>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>>  			 dev_priv->csr.fw_path,
>>  			 CSR_VERSION_MAJOR(csr->version),
>> @@ -440,6 +438,7 @@ static void csr_load_work_fn(struct work_struct *work)
>>  			   INTEL_UC_FIRMWARE_URL);
>>  	}
>>  
>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>  	release_firmware(fw);
>>  }
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  8:15   ` Lee, Shawn C
@ 2018-11-21  9:05     ` Jani Nikula
  2018-11-21  9:17       ` Lee, Shawn C
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2018-11-21  9:05 UTC (permalink / raw)
  To: Lee, Shawn C, intel-gfx; +Cc: Chiou, Cooper, Vivi, Rodrigo

On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>> Driver obtain power well at intel_csr_ucode_init().
>>> And release it after load DMC firmware successful.
>>
>>Correct.
>>
>>> An issue happened when DMC was not found or failed to load. Power well 
>>> would not be released and just output some error messages. Driver have 
>>> to release power well properly to keep put/get balance.
>>
>>No. We intentionally do not release it until dmc firmware load succeeds.
>
> If load DMC failed, we found DP phy was always on even without
> external display connected.  So it looks like an expected behavior,
> right?

I'll put it this way, we don't really go out of our way to support
everything without the DMC firmware. Every choice like this doubles the
testing requirements.

Do you see issues with DMC firmware loaded? Do you have issues with
loading DMC firmware?

BR,
Jani.


>
>>
>>See the comment in intel_csr_ucode_init(), as well as this in the branch where dmc load fails:
>>
>>		dev_notice(dev_priv->drm.dev,
>>			   "Failed to load DMC firmware %s."
>>			   " Disabling runtime power management.\n",
>>			   csr->fw_path);
>>
>>We don't support runtime pm without dmc on platforms with dmc.
>>
>>BR,
>>Jani.
>>
>>>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>>
>>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c 
>>> b/drivers/gpu/drm/i915/intel_csr.c
>>> index a516697bf57d..8d04d7b6f00a 100644
>>> --- a/drivers/gpu/drm/i915/intel_csr.c
>>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>>> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>>>  	if (dev_priv->csr.dmc_payload) {
>>>  		intel_csr_load_program(dev_priv);
>>>  
>>> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>> -
>>>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>>>  			 dev_priv->csr.fw_path,
>>>  			 CSR_VERSION_MAJOR(csr->version),
>>> @@ -440,6 +438,7 @@ static void csr_load_work_fn(struct work_struct *work)
>>>  			   INTEL_UC_FIRMWARE_URL);
>>>  	}
>>>  
>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>  	release_firmware(fw);
>>>  }
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  9:05     ` Jani Nikula
@ 2018-11-21  9:17       ` Lee, Shawn C
  2018-11-21  9:29         ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Lee, Shawn C @ 2018-11-21  9:17 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx; +Cc: Chiou, Cooper, Vivi, Rodrigo

On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>>> Driver obtain power well at intel_csr_ucode_init().
>>>> And release it after load DMC firmware successful.
>>>
>>>Correct.
>>>
>>>> An issue happened when DMC was not found or failed to load. Power 
>>>> well would not be released and just output some error messages. 
>>>> Driver have to release power well properly to keep put/get balance.
>>>
>>>No. We intentionally do not release it until dmc firmware load succeeds.
>>
>> If load DMC failed, we found DP phy was always on even without 
>> external display connected.  So it looks like an expected behavior, 
>> right?
>
>I'll put it this way, we don't really go out of our way to support everything without the DMC firmware. Every choice like this doubles the testing requirements.
>

Understood. This is not a normal case (without DMC) on customer system.
We just think it will be better to release power well if we already got it before.

>Do you see issues with DMC firmware loaded? Do you have issues with loading DMC firmware?

No issue with DMC firmware loaded. Thanks!

>
>BR,
>Jani.
>
>
>>
>>>
>>>See the comment in intel_csr_ucode_init(), as well as this in the branch where dmc load fails:
>>>
>>>		dev_notice(dev_priv->drm.dev,
>>>			   "Failed to load DMC firmware %s."
>>>			   " Disabling runtime power management.\n",
>>>			   csr->fw_path);
>>>
>>>We don't support runtime pm without dmc on platforms with dmc.
>>>
>>>BR,
>>>Jani.
>>>
>>>>
>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>>>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>>>
>>>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c
>>>> b/drivers/gpu/drm/i915/intel_csr.c
>>>> index a516697bf57d..8d04d7b6f00a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_csr.c
>>>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>>>> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>>>>  	if (dev_priv->csr.dmc_payload) {
>>>>  		intel_csr_load_program(dev_priv);
>>>>  
>>>> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>> -
>>>>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>>>>  			 dev_priv->csr.fw_path,
>>>>  			 CSR_VERSION_MAJOR(csr->version), @@ -440,6 +438,7 @@ static 
>>>> void csr_load_work_fn(struct work_struct *work)
>>>>  			   INTEL_UC_FIRMWARE_URL);
>>>>  	}
>>>>  
>>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>>  	release_firmware(fw);
>>>>  }
>>>
>>>--
>>>Jani Nikula, Intel Open Source Graphics Center
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  9:17       ` Lee, Shawn C
@ 2018-11-21  9:29         ` Jani Nikula
  2018-11-22  4:49           ` Lee, Shawn C
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2018-11-21  9:29 UTC (permalink / raw)
  To: Lee, Shawn C, intel-gfx; +Cc: Chiou, Cooper, Vivi, Rodrigo

On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>>> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>>>> Driver obtain power well at intel_csr_ucode_init().
>>>>> And release it after load DMC firmware successful.
>>>>
>>>>Correct.
>>>>
>>>>> An issue happened when DMC was not found or failed to load. Power 
>>>>> well would not be released and just output some error messages. 
>>>>> Driver have to release power well properly to keep put/get balance.
>>>>
>>>>No. We intentionally do not release it until dmc firmware load succeeds.
>>>
>>> If load DMC failed, we found DP phy was always on even without 
>>> external display connected.  So it looks like an expected behavior, 
>>> right?
>>
>>I'll put it this way, we don't really go out of our way to support everything without the DMC firmware. Every choice like this doubles the testing requirements.
>>
>
> Understood. This is not a normal case (without DMC) on customer
> system.  We just think it will be better to release power well if we
> already got it before.

Why are you supporting a non-DMC setup on a customer system? Don't do
it.

BR,
Jani.

>
>>Do you see issues with DMC firmware loaded? Do you have issues with loading DMC firmware?
>
> No issue with DMC firmware loaded. Thanks!
>
>>
>>BR,
>>Jani.
>>
>>
>>>
>>>>
>>>>See the comment in intel_csr_ucode_init(), as well as this in the branch where dmc load fails:
>>>>
>>>>		dev_notice(dev_priv->drm.dev,
>>>>			   "Failed to load DMC firmware %s."
>>>>			   " Disabling runtime power management.\n",
>>>>			   csr->fw_path);
>>>>
>>>>We don't support runtime pm without dmc on platforms with dmc.
>>>>
>>>>BR,
>>>>Jani.
>>>>
>>>>>
>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>>>>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>>>>
>>>>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c
>>>>> b/drivers/gpu/drm/i915/intel_csr.c
>>>>> index a516697bf57d..8d04d7b6f00a 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_csr.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>>>>> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>>>>>  	if (dev_priv->csr.dmc_payload) {
>>>>>  		intel_csr_load_program(dev_priv);
>>>>>  
>>>>> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>>> -
>>>>>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>>>>>  			 dev_priv->csr.fw_path,
>>>>>  			 CSR_VERSION_MAJOR(csr->version), @@ -440,6 +438,7 @@ static 
>>>>> void csr_load_work_fn(struct work_struct *work)
>>>>>  			   INTEL_UC_FIRMWARE_URL);
>>>>>  	}
>>>>>  
>>>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>>>  	release_firmware(fw);
>>>>>  }
>>>>
>>>>--
>>>>Jani Nikula, Intel Open Source Graphics Center
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Release power well if load DMC failed
  2018-11-21  7:59 [PATCH] drm/i915: Release power well if load DMC failed Lee, Shawn C
  2018-11-21  7:53 ` Jani Nikula
  2018-11-21  8:01 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-11-21 13:46 ` Patchwork
  2 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-11-21 13:46 UTC (permalink / raw)
  To: Lee, Shawn C; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Release power well if load DMC failed
URL   : https://patchwork.freedesktop.org/series/52805/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5175_full -> Patchwork_10875_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10875_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10875_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10875_full:

  === IGT changes ===

    ==== Warnings ====

    igt@tools_test@sysfs_l3_parity:
      shard-hsw:          SKIP -> PASS

    igt@tools_test@tools_test:
      {shard-iclb}:       SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10875_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_eio@in-flight-suspend:
      shard-glk:          PASS -> DMESG-WARN (fdo#107957)

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108074)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_cursor_crc@cursor-128x42-random:
      shard-glk:          PASS -> FAIL (fdo#103232) +3

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108) +1

    igt@kms_draw_crc@draw-method-xrgb8888-blt-xtiled:
      shard-skl:          PASS -> FAIL (fdo#107791)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
      shard-skl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      {shard-iclb}:       PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
      shard-glk:          PASS -> FAIL (fdo#103167) +7

    igt@kms_frontbuffer_tracking@fbcpsr-1p-shrfb-fliptrack:
      shard-skl:          PASS -> FAIL (fdo#105682) +1

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      {shard-iclb}:       PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166) +2

    igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107724) +1

    igt@kms_setmode@basic:
      shard-hsw:          PASS -> FAIL (fdo#99912)

    igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
      shard-skl:          SKIP -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_chv_cursor_fail@pipe-b-256x256-right-edge:
      shard-skl:          FAIL (fdo#104671) -> PASS

    igt@kms_color@pipe-c-ctm-0-75:
      shard-skl:          FAIL (fdo#108682) -> PASS

    igt@kms_cursor_crc@cursor-256x85-onscreen:
      shard-glk:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#106509, fdo#105454) -> PASS

    igt@kms_draw_crc@draw-method-xrgb8888-render-ytiled:
      {shard-iclb}:       WARN (fdo#108336) -> PASS +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      {shard-iclb}:       FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
      {shard-iclb}:       DMESG-FAIL (fdo#107724) -> PASS +6

    igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
      shard-skl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      {shard-iclb}:       FAIL (fdo#103166) -> PASS +1

    igt@kms_rotation_crc@primary-rotation-180:
      {shard-iclb}:       DMESG-WARN (fdo#107724, fdo#108336) -> PASS +10

    igt@kms_sequence@get-busy:
      {shard-iclb}:       DMESG-WARN (fdo#107724) -> PASS +9

    igt@pm_rpm@dpms-mode-unset-non-lpsp:
      shard-skl:          INCOMPLETE (fdo#107807) -> SKIP

    igt@pm_rpm@system-suspend-devices:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    
    ==== Warnings ====

    igt@kms_cursor_crc@cursor-128x128-suspend:
      {shard-iclb}:       DMESG-FAIL (fdo#103232, fdo#107724) -> FAIL (fdo#103232)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107791 https://bugs.freedesktop.org/show_bug.cgi?id=107791
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#107957 https://bugs.freedesktop.org/show_bug.cgi?id=107957
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108336 https://bugs.freedesktop.org/show_bug.cgi?id=108336
  fdo#108682 https://bugs.freedesktop.org/show_bug.cgi?id=108682
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (7 -> 7) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5175 -> Patchwork_10875

  CI_DRM_5175: eccbba7017aafd70e09bb105c8a6b85572a93eb8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4723: 53bb24ad410b53cdd96f15ced8fd5921c8ab0eac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10875: 285be726370fd7560644f653b3428ab4f66af9fb @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10875/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Release power well if load DMC failed
  2018-11-21  9:29         ` Jani Nikula
@ 2018-11-22  4:49           ` Lee, Shawn C
  0 siblings, 0 replies; 9+ messages in thread
From: Lee, Shawn C @ 2018-11-22  4:49 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx; +Cc: Chiou, Cooper, Vivi, Rodrigo

On Wed, 21 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>>On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>>>> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula@intel.com> wrote:
>>>>>> Driver obtain power well at intel_csr_ucode_init().
>>>>>> And release it after load DMC firmware successful.
>>>>>
>>>>>Correct.
>>>>>
>>>>>> An issue happened when DMC was not found or failed to load. Power 
>>>>>> well would not be released and just output some error messages.
>>>>>> Driver have to release power well properly to keep put/get balance.
>>>>>
>>>>>No. We intentionally do not release it until dmc firmware load succeeds.
>>>>
>>>> If load DMC failed, we found DP phy was always on even without 
>>>> external display connected.  So it looks like an expected behavior, 
>>>> right?
>>>
>>>I'll put it this way, we don't really go out of our way to support everything without the DMC firmware. Every choice like this doubles the testing requirements.
>>>
>>
>> Understood. This is not a normal case (without DMC) on customer 
>> system.  We just think it will be better to release power well if we 
>> already got it before.
>
>Why are you supporting a non-DMC setup on a customer system? Don't do it.
>

As we mention before. It is an abnormal case that DMC was lost on customer system.
Then we got an issue like this. Everything works normally if DMC is available.
Thanks for comments!

>BR,
>Jani.
>
>>
>>>Do you see issues with DMC firmware loaded? Do you have issues with loading DMC firmware?
>>
>> No issue with DMC firmware loaded. Thanks!
>>
>>>
>>>BR,
>>>Jani.
>>>
>>>
>>>>
>>>>>
>>>>>See the comment in intel_csr_ucode_init(), as well as this in the branch where dmc load fails:
>>>>>
>>>>>		dev_notice(dev_priv->drm.dev,
>>>>>			   "Failed to load DMC firmware %s."
>>>>>			   " Disabling runtime power management.\n",
>>>>>			   csr->fw_path);
>>>>>
>>>>>We don't support runtime pm without dmc on platforms with dmc.
>>>>>
>>>>>BR,
>>>>>Jani.
>>>>>
>>>>>>
>>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>>>>>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>>>>>
>>>>>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/i915/intel_csr.c | 3 +--
>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c
>>>>>> b/drivers/gpu/drm/i915/intel_csr.c
>>>>>> index a516697bf57d..8d04d7b6f00a 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_csr.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>>>>>> @@ -425,8 +425,6 @@ static void csr_load_work_fn(struct work_struct *work)
>>>>>>  	if (dev_priv->csr.dmc_payload) {
>>>>>>  		intel_csr_load_program(dev_priv);
>>>>>>  
>>>>>> -		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>>>> -
>>>>>>  		DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>>>>>>  			 dev_priv->csr.fw_path,
>>>>>>  			 CSR_VERSION_MAJOR(csr->version), @@ -440,6 +438,7 @@ static 
>>>>>> void csr_load_work_fn(struct work_struct *work)
>>>>>>  			   INTEL_UC_FIRMWARE_URL);
>>>>>>  	}
>>>>>>  
>>>>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>>>>>>  	release_firmware(fw);
>>>>>>  }
>>>>>
>>>>>--
>>>>>Jani Nikula, Intel Open Source Graphics Center
>>>
>>>--
>>>Jani Nikula, Intel Open Source Graphics Center
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-11-22  4:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21  7:59 [PATCH] drm/i915: Release power well if load DMC failed Lee, Shawn C
2018-11-21  7:53 ` Jani Nikula
2018-11-21  8:15   ` Lee, Shawn C
2018-11-21  9:05     ` Jani Nikula
2018-11-21  9:17       ` Lee, Shawn C
2018-11-21  9:29         ` Jani Nikula
2018-11-22  4:49           ` Lee, Shawn C
2018-11-21  8:01 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-11-21 13:46 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.