[PATCH v8] drm/i915/dmc: Create debugfs entry for dc6 counter
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Mon Mar 24 09:11:12 UTC 2025
On 3/24/2025 2:08 PM, Jani Nikula wrote:
> On Mon, 24 Mar 2025, "Nautiyal, Ankit K" <ankit.k.nautiyal at intel.com> wrote:
>> On 3/21/2025 6:07 PM, 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.
>>>
>>> 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.
>>>
>>> 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_counter += dc5_current_count - dc5_start_count
>>>
>>> v2: Update the discription. (Imre)
>>> Read dc5 count during dc6 enable and disable then and update
>>> dc6 residency counter. (Imre)
>>> Remove variable from dmc structure. (Jani)
>>> Updated the subject title.
>>> v3: Add i915_power_domains lock to updated dc6 count in debugfs. (Imre)
>>> Use flags to check dc6 enable/disable states. (Imre)
>>> Move the display version check and counter read/update to
>>> a helper. (Imre)
>>> Resize the variable length. (Rodrigo)
>>> Use old dc6 debugfs entry for every platform. (Rodrigo)
>>> v4: Remove superfluous whitespace. (Jani)
>>> Read DMC registers in intel_dmc.c (Jani)
>>> Rename dc6_en_dis to dc6_enabled and change its type to bool. (Jani)
>>> Rename update_dc6_count and move it to intel_dmc.c (Jani)
>>> Rename dc6_en_dis to start_tracking. (Imre)
>>> Have lock for dc6 state read aswelll. (Imre)
>>> Keep the existing way print 'DC5 -> DC6 count' along with
>>> new 'DC6 Allowed Count' print. (Imre)
>>> Add counters in intel_dmc struct. (Imre)
>>> Have interface to return dc6 allowed count. (Imre)
>>> Rename dc6_count to dc6_allowed_count. (Rodrigo)
>>> v5: Rename counters and move in to dc6_allowed structure. (Imre)
>>> Order declaration lines in decreasing line length. (Imre)
>>> Update start_tacking logic. (Imre)
>>> Move get couner inside lock and DISPLAY_VER code to helper. (Imre)
>>> v6: Change intel_dmc_get_dc6_allowed_count return type to bool. (Imre)
>>> Update debugfs print to better allien with old print. (Imre)
>>> Remove braces at if/else for signle line statements. (Imre)
>>> v7: Remove in line variable declaration. (Imre)
>>> v8: Rebase the changes.
>>>
>>> Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
>>> Reviewed-by: Imre Deak <imre.deak at intel.com>
>> Thanks for the patch and the reviews, patch is pushed to drm-intel-next
> Where are the CI results? We don't push stuff without CI results.
We had Full CI results with https://patchwork.freedesktop.org/series/144240/
There was some conflict due to a header file, for which the patch was
rebased and sent again.
But yes, I could have waited for FULL CI results on this patch. Will
take care of this going forward.
Regards,
Ankit
>
> BR,
> Jani.
>
>>
>> Regards,
>>
>> Ankit
>>
>>> ---
>>> .../i915/display/intel_display_power_well.c | 11 +++-
>>> drivers/gpu/drm/i915/display/intel_dmc.c | 50 ++++++++++++++++++-
>>> drivers/gpu/drm/i915/display/intel_dmc.h | 1 +
>>> 3 files changed, 60 insertions(+), 2 deletions(-)
>>>
>>> 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 751e49b880d6..b9b4359751cc 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>>> @@ -749,8 +749,9 @@ void gen9_sanitize_dc_state(struct intel_display *display)
>>> void gen9_set_dc_state(struct intel_display *display, u32 state)
>>> {
>>> struct i915_power_domains *power_domains = &display->power.domains;
>>> - u32 val;
>>> + bool dc6_was_enabled, enable_dc6;
>>> u32 mask;
>>> + u32 val;
>>>
>>> if (!HAS_DISPLAY(display))
>>> return;
>>> @@ -769,11 +770,19 @@ void gen9_set_dc_state(struct intel_display *display, u32 state)
>>> drm_err(display->drm, "DC state mismatch (0x%x -> 0x%x)\n",
>>> power_domains->dc_state, val & mask);
>>>
>>> + enable_dc6 = state & DC_STATE_EN_UPTO_DC6;
>>> + dc6_was_enabled = val & DC_STATE_EN_UPTO_DC6;
>>> + if (!dc6_was_enabled && enable_dc6)
>>> + intel_dmc_update_dc6_allowed_count(display, true);
>>> +
>>> val &= ~mask;
>>> val |= state;
>>>
>>> gen9_write_dc_state(display, val);
>>>
>>> + if (!enable_dc6 && dc6_was_enabled)
>>> + intel_dmc_update_dc6_allowed_count(display, false);
>>> +
>>> power_domains->dc_state = val & mask;
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>>> index eb6b47ba0870..98f80a6c63e8 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>>> @@ -29,6 +29,7 @@
>>> #include "i915_reg.h"
>>> #include "intel_de.h"
>>> #include "intel_display_rpm.h"
>>> +#include "intel_display_power_well.h"
>>> #include "intel_dmc.h"
>>> #include "intel_dmc_regs.h"
>>> #include "intel_step.h"
>>> @@ -58,6 +59,10 @@ struct intel_dmc {
>>> const char *fw_path;
>>> u32 max_fw_size; /* bytes */
>>> u32 version;
>>> + struct {
>>> + u32 dc5_start;
>>> + u32 count;
>>> + } dc6_allowed;
>>> struct dmc_fw_info {
>>> u32 mmio_count;
>>> i915_reg_t mmioaddr[20];
>>> @@ -1233,6 +1238,44 @@ void intel_dmc_snapshot_print(const struct intel_dmc_snapshot *snapshot, struct
>>> DMC_VERSION_MINOR(snapshot->version));
>>> }
>>>
>>> +void intel_dmc_update_dc6_allowed_count(struct intel_display *display,
>>> + bool start_tracking)
>>> +{
>>> + struct intel_dmc *dmc = display_to_dmc(display);
>>> + u32 dc5_cur_count;
>>> +
>>> + if (DISPLAY_VER(dmc->display) < 14)
>>> + return;
>>> +
>>> + dc5_cur_count = intel_de_read(dmc->display, DG1_DMC_DEBUG_DC5_COUNT);
>>> +
>>> + if (!start_tracking)
>>> + dmc->dc6_allowed.count += dc5_cur_count - dmc->dc6_allowed.dc5_start;
>>> +
>>> + dmc->dc6_allowed.dc5_start = dc5_cur_count;
>>> +}
>>> +
>>> +static bool intel_dmc_get_dc6_allowed_count(struct intel_display *display, u32 *count)
>>> +{
>>> + struct i915_power_domains *power_domains = &display->power.domains;
>>> + struct intel_dmc *dmc = display_to_dmc(display);
>>> + bool dc6_enabled;
>>> +
>>> + if (DISPLAY_VER(display) < 14)
>>> + return false;
>>> +
>>> + mutex_lock(&power_domains->lock);
>>> + dc6_enabled = intel_de_read(display, DC_STATE_EN) &
>>> + DC_STATE_EN_UPTO_DC6;
>>> + if (dc6_enabled)
>>> + intel_dmc_update_dc6_allowed_count(display, false);
>>> +
>>> + *count = dmc->dc6_allowed.count;
>>> + mutex_unlock(&power_domains->lock);
>>> +
>>> + return true;
>>> +}
>>> +
>>> static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
>>> {
>>> struct intel_display *display = m->private;
>>> @@ -1240,6 +1283,7 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
>>> struct intel_dmc *dmc = display_to_dmc(display);
>>> struct ref_tracker *wakeref;
>>> i915_reg_t dc5_reg, dc6_reg = INVALID_MMIO_REG;
>>> + u32 dc6_allowed_count;
>>>
>>> if (!HAS_DMC(display))
>>> return -ENODEV;
>>> @@ -1288,7 +1332,11 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
>>> }
>>>
>>> seq_printf(m, "DC3 -> DC5 count: %d\n", intel_de_read(display, dc5_reg));
>>> - if (i915_mmio_reg_valid(dc6_reg))
>>> +
>>> + if (intel_dmc_get_dc6_allowed_count(display, &dc6_allowed_count))
>>> + seq_printf(m, "DC5 -> DC6 allowed count: %d\n",
>>> + dc6_allowed_count);
>>> + else if (i915_mmio_reg_valid(dc6_reg))
>>> seq_printf(m, "DC5 -> DC6 count: %d\n",
>>> intel_de_read(display, dc6_reg));
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
>>> index 44cecef98e73..c78426eb4cd5 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dmc.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
>>> @@ -26,6 +26,7 @@ void intel_dmc_debugfs_register(struct intel_display *display);
>>>
>>> struct intel_dmc_snapshot *intel_dmc_snapshot_capture(struct intel_display *display);
>>> void intel_dmc_snapshot_print(const struct intel_dmc_snapshot *snapshot, struct drm_printer *p);
>>> +void intel_dmc_update_dc6_allowed_count(struct intel_display *display, bool start_tracking);
>>>
>>> void assert_dmc_loaded(struct intel_display *display);
>>>
More information about the Intel-gfx
mailing list