Re: [PATCH] drm/i915/dmc: Add debugfs for dc6 counter

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

On Mon, Feb 03, 2025 at 02:26:13PM +0530, Mohammed Thasleem wrote:
> Starting from MTl we don't have a platform agnostic way to validate DC6 state
> due to dc6 counter has been removed to validate DC state.
> Adding dc6_entry_counter at display dirver to validate dc6 state.
> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_core.h       | 1 +
>  drivers/gpu/drm/i915/display/intel_display_power_well.c | 2 ++
>  drivers/gpu/drm/i915/display/intel_dmc.c                | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 554870d2494b..cc244617011f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -376,6 +376,7 @@ struct intel_display {
>  	struct {
>  		struct intel_dmc *dmc;
>  		intel_wakeref_t wakeref;
> +		u32 dc6_entry_counter;
>  	} dmc;
>  
>  	struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index f45a4f9ba23c..0eb178aa618d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -869,6 +869,8 @@ void skl_enable_dc6(struct intel_display *display)
>  	intel_dmc_wl_enable(display, DC_STATE_EN_UPTO_DC6);
>  
>  	gen9_set_dc_state(display, DC_STATE_EN_UPTO_DC6);
> +
> +	display->dmc.dc6_entry_counter++;

AFAIU the goal is to validate that the display HW can reach the DC6
power state. There is no HW DC6 residency counter (and there wasn't such
a counter earlier either), so an alternative way is required. According
to the HW team the display driver has programmed everything correctly in
order to allow the DC6 power state if the DC5 power state is reached
(indicated by the HW DC5 residency counter incrementing) and DC6 is
enabled by the driver.

Based on the above, we'd need a DC6 residency counter maintained by the
driver which is incremented if the DC5 residency counter increments
while DC6 is enabled. The dc6_entry_counter in this patch is not enough
for this, since it doesn't take into account the DC5 residency. While
user space could check both dc6_entry_counter and the DC5 residency,
that check would be racy wrt. the driver enabling/disabling the DC6
state asynchronously.

I suppose the driver could take a snapshot of the DC5 residency counter
right after it enables DC6 (dc5_residency_start) and increment the SW
DC6 residency counter right before it disables DC6 or when user space
reads the DC6 counter. So the driver would update the counter at these
two points in the following way:
dc6_residency += dc5_residency_current - dc5_residency_start

The commit log would need a justification, something along the above
lines.

>  }
>  
>  void bxt_enable_dc9(struct intel_display *display)
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 221d3abda791..f51bd8e6011d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -1293,6 +1293,7 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
>  	if (i915_mmio_reg_valid(dc6_reg))
>  		seq_printf(m, "DC5 -> DC6 count: %d\n",
>  			   intel_de_read(display, dc6_reg));
> +	seq_printf(m, "DC6 entry count: %d\n", display->dmc.dc6_entry_counter);
>  
>  	seq_printf(m, "program base: 0x%08x\n",
>  		   intel_de_read(display, DMC_PROGRAM(dmc->dmc_info[DMC_FW_MAIN].start_mmioaddr, 0)));
> -- 
> 2.43.0
>