[Intel-gfx] [PATCH] drm/i915: Release power well if load DMC failed

[Lee, Shawn C]

On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula at intel.com> wrote:
>On Wed, 21 Nov 2018, "Lee, Shawn C" <shawn.c.lee at intel.com> wrote:
>> On Tue, 20 Nov 2018, "Jani Nikula" <jani.nikula at 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 at intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>> Cc: Jose Roberto de Souza <jose.souza at intel.com>
>>>> Cc: Cooper Chiou <cooper.chiou at intel.com>
>>>>
>>>> Signed-off-by: Lee, Shawn C <shawn.c.lee at 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