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

[Imre Deak <imre.deak@intel.com>]

On Thu, Mar 13, 2025 at 01:02:09AM +0530, Naladala, Ramanaidu wrote:
> 
> 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.

We are using the DG1_DMC_DEBUG_DC5_COUNT (0x134154) register to check
for DC5 _entries_. This matches the bspec (49786) definition of the
register:

"Residency counter for DC5 state. Indicates the number of DC5 entries."

In fact all other DMC_DEBUG_ registers are specified in a similar way,
indicating the residency for a given state, that is entries. Counting
both entries and exits in the same counter would be ultimately broken in
any case.

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