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

[Jani Nikula]

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:
>>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.

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 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

-- 
Jani Nikula, Intel Open Source Graphics Center