Re: [SKL-DMC-BUGFIX 4/5] drm/i915/skl: Block disable call for pw1 if dmc firmware is present.

[Animesh Manna <animesh.manna@intel.com>]

On 8/5/2015 2:44 PM, Daniel Vetter wrote:
> On Mon, Aug 03, 2015 at 09:55:35PM +0530, Animesh Manna wrote:
>> Another interesting criteria to work dmc as expected is pw1 to be
>> enabled by driver and dmc will shut it off in its execution
>> sequence. If already disabled by driver dmc will get confuse and
>> behave differently than expected found during pc10 entry issue
>> for skl.
>>
>> So berfore we disable power-well 1, added check if dmc firmware is
>> present and driver will not disable power well 1, but for any reason
>> if firmware is not present of failed to load we can shut off the
>> power well 1 which will save some power.
>>
>> As skl is currently fully dependent on dmc to go in lowest possible
>> power state (dc6) but the same is not applicable for bxt. Display
>> engine can enter into dc9 without dmc, hence unblocking disable call.
>>
>> v1: Initial version.
>>
>> v2: Based on revire commnents from Sunil,
>> - condition check for pw1 is moved in skl_set_power_well.
> There's another patch from Damien/Paulo to do some similar fumbling
> between LCPLL and PW1. We probably want to completely take away PW1 from
> being controlled by the power wells code and push it all into the rpm code
> (where we either disable pw1, pw-misc and lcpll in one go or leave it all
> to the dmc firmware).
> -Daniel

Patch from Damien/Paulo submitted in intel-gfx mailing list? I have not seen the
actual implementation.

skl_set_power_well() is the function where enable/disable operation for all power-wells
is implemented. Imo, till we can add different condition check for taking care
special cases like dmc, its better to put power-well related enable/disable
code in same place - by this we will avoid code duplication and code readability
will be better. Send me your suggestion, accordingly if needed I will add required
changes.

- Animesh

>
>> 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>
>> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index c660245..00cd4ff 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -636,9 +636,15 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>>   		}
>>   	} else {
>>   		if (enable_requested) {
>> -			I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
>> -			POSTING_READ(HSW_PWR_WELL_DRIVER);
>> -			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>> +			if (IS_SKYLAKE(dev) &&
>> +				(power_well->data == SKL_DISP_PW_1) &&
>> +				(intel_csr_load_status_get(dev_priv) == FW_LOADED))
>> +				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
>> +			else {
>> +				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
>> +				POSTING_READ(HSW_PWR_WELL_DRIVER);
>> +				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>> +			}
>>   
>>   			if (GEN9_ENABLE_DC5(dev) &&
>>   				power_well->data == SKL_DISP_PW_2)
>> -- 
>> 2.0.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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