Re: [PATCH v5] drm/i915/dmc: Create debugfs entry for dc6 counter

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

On 3/13/2025 12:19 AM, Imre Deak wrote:
> On Wed, Mar 12, 2025 at 11:44:20PM +0530, Naladala, Ramanaidu wrote:
>> [...]
>>>> +static u32 intel_dmc_get_dc6_allowed_count(struct intel_display *display, u32 *count)
>>> The return type isn't compatible with the -ENODEV returned value. I'd
>>> just return a bool, since the reason for an error is always the same.
>>>
>>>> +{
>>>> +	struct i915_power_domains *power_domains = &display->power.domains;
>>>> +	struct intel_dmc *dmc = display_to_dmc(display);
>>>> +
>>>> +	if (DISPLAY_VER(display) < 14)
>>>> +		return -ENODEV;
>>>> +
>>>> +	mutex_lock(&power_domains->lock);
>>>> +	bool dc6_enabled = DC_STATE_EN_UPTO_DC6 &
>>>> +			   intel_de_read(display, DC_STATE_EN);
>> The dc6_enabled flag indicates only the DC state limit. If all conditions
>> are met, the DMC can entry/exits DC6.
>>
>> However, if the DC6 conditions are not met, the DMC can perform entry/exits
>> up to DC5. Entry/exits from DC5 to DC3 can also change the DG1_DMC_DEBUG_DC5_COUNT
>> counter values. It is better to add a pc10 check along with the dc6_enabled flag.
>>
>> Correct me if my understanding is wrong.
> According to HW people, the conditions for DC6 are met from the
> _display_ side if the conditions for DC5 are met and DC6 is enabled. The
> problem of making this dependent on package C states is that those
> states also depend on non-display IPs. The purpose of this counter (DC6
> allowed) is validating the display driver's DC6 programming, without
> depending on the validity of the programming for all other IPs (by
> non-display drivers) that could block actual DC6 transitions.

In that case, we have the DC3 entry/exit counter 
DG1_DMC_DEBUG_DC1_COUNT. Add a check to ensure it does not change when 
the DC5 counters are changing.

It will confirm the transaction entry/exits are between DC5 and DC6. If 
DC3 counter is changed, don't increment the dc6 counter value.


>
>>>> +	if (dc6_enabled)
>>>> +		intel_dmc_update_dc6_allowed_count(display, false);
>>>> +
>>>> +	*count = dmc->dc6_allowed.count;
>>>> +	mutex_unlock(&power_domains->lock);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +