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;
> > > +}
> > > +
