On Mon, Feb 03, 2025 at 12:15:26PM -0500, Rodrigo Vivi wrote: > > > > [...] > > > > > > > > The driver enabling DC6 is not an enough condition for DC6 being allowed > > > > from the display side. Some display clock gating etc. configuration by > > > > the driver could be blocking it. According to the HW team, DC5 being > > > > entered while DC6 is enabled is a guarantee that DC6 is allowed from the > > > > display side - i.e. the driver has configured everything correctly for > > > > that. > > > > > > Fair enough. So IGT test case would check directly if DC5 counter is > > > increasing and DC6 is allowed. > > > > > > Something as simple as this in the kernel code would tell that > > > DC6 is enabled: > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > > > @@ -1294,6 +1294,10 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused) > > > seq_printf(m, "DC5 -> DC6 count: %d\n", > > > intel_de_read(display, dc6_reg)); > > > > > > + seq_printf(m, "DC6 allowed: %s\n", str_yes_no((intel_de_read(display, > > > + DC_STATE_EN) > > > + & 0x3) == 2)); > > > + > > > > > > and > > > > > > $ cat i915_dmc_info > > > [snip] > > > DC3 -> DC5 count: 286 > > > DC5 -> DC6 count: 0 > > > DC6 allowed: yes > > > [snip] > > > > > > $ cat i915_dmc_info > > > [snip] > > > DC3 -> DC5 count: 292 > > > DC5 -> DC6 count: 0 > > > DC6 allowed: yes > > > [snip] > > > > > > Thoughts? > > > > The DC5 increment could've happened while DC6 was disabled by the driver. > > Yes, it could... in general the dc6 bit would be zero, but right, there's > a possible race. > > But should we complicate things when we know that on the test case itself > we are in full control of the modeset?! From the test perspective I believe > this would be more than enough without complicating things. Imo having a counter which works based on the condition that guarantees the display to allow DC6, would help later in debugging. > But well, if you really believe that we need to go further in the driver > to cover that it is fine by me. > > But just to ensure that we are in the same page, could you please open > up a bit more of your idea on when to increase the dc5 sw counters > in a way that we would ensure that we don't have race and that we > get the dc6 allowed counter correctly? Something like the following: 1. Right after the driver sets DC6_EN, it stores the DC5 HW counter's current value: dc5_start = dc5_current 2. Right before the driver clears DC6_EN, it updates the DC6 allowed counter: dc6_allowed += dc5_current - dc5_start dc5_start = dc5_current 3. When userspace reads the counters via debugfs the driver first updates dc6_allowed the same way as 2. did if DC6_EN is set. > Btw, while doing this experiment I noticed that the debugfs and test > also doesn't call that residency anyway and it is just count. So > perhaps with your idea we don't need to change the debugfs interface.
