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

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

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.

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