Re: [PATCH] amd/display/debugfs: add sysfs entry to read PSR residency from firmware

["S, Shirish" <sshankar@xxxxxxx>]

On 3/10/2023 12:00 PM, S, Shirish wrote:
> On 3/8/2023 11:52 PM, Hamza Mahfooz wrote:
> > On 3/8/23 02:10, Shirish S wrote:
> > > [Why]
> > > Currently there aren't any methods to determine PSR state residency.
> > >
> > > [How]
> > > create a sysfs entry for reading residency and internally hook it up
> > > to existing functionality of reading PSR residency from firmware.
> > >
> > > Signed-off-by: Shirish S <shirish.s@xxxxxxx>
> > > ---
> > >   .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 19 
> > > +++++++++++++++++++
> > >   1 file changed, 19 insertions(+)
> > >
> > > diff --git 
> > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
> > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > > index abf7895d1608..8ff2802db5b5 100644
> > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > > @@ -27,6 +27,7 @@
> > >   #include <linux/uaccess.h>
> > >     #include "dc.h"
> > > +#include "dc_link.h"
> >
> > Please drop this include, the relevant function should already be
> > accessible from dc.h.
>
> Good catch. Removed and respun the patch 
> (https://patchwork.freedesktop.org/patch/526211/)
>
> Pls review.
>
> Regards,
>
> Shirish S

Well, the code structure has been changed now since : "0078c924e733 
drm/amd/display: move eDP panel control logic to link_edp_panel_control"

Now "dc.h" no more includes "link.h".


> > >   #include "amdgpu.h"
> > >   #include "amdgpu_dm.h"
> > >   #include "amdgpu_dm_debugfs.h"
> > > @@ -2793,6 +2794,22 @@ static int psr_get(void *data, u64 *val)
> > >       return 0;
> > >   }
> > >   +/*
> > > + *  Read PSR state residency
> > > + */
> > > +static int psr_read_residency(void *data, u64 *val)
> > > +{
> > > +    struct amdgpu_dm_connector *connector = data;
> > > +    struct dc_link *link = connector->dc_link;
> > > +    u32 residency;
> > > +
> > > +    dc_link_get_psr_residency(link, &residency);
> >
> > Did you mean to use link_get_psr_residency() here?

Yes, my code was a bit older, have incorporated final changes in new 
patch: https://patchwork.freedesktop.org/patch/526213/

Pls review.

Regards,

Shirish S

> > > +
> > > +    *val = (u64)residency;
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >   /*
> > >    * Set dmcub trace event IRQ enable or disable.
> > >    * Usage to enable dmcub trace event IRQ: echo 1 > 
> > > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
> > > @@ -2828,6 +2845,7 @@ 
> > > DEFINE_DEBUGFS_ATTRIBUTE(dmcub_trace_event_state_fops, 
> > > dmcub_trace_event_state_g
> > >                dmcub_trace_event_state_set, "%llu\n");
> > >     DEFINE_DEBUGFS_ATTRIBUTE(psr_fops, psr_get, NULL, "%llu\n");
> > > +DEFINE_DEBUGFS_ATTRIBUTE(psr_residency_fops, psr_read_residency, 
> > > NULL, "%llu\n");
> > >     DEFINE_SHOW_ATTRIBUTE(current_backlight);
> > >   DEFINE_SHOW_ATTRIBUTE(target_backlight);
> > > @@ -2991,6 +3009,7 @@ void connector_debugfs_init(struct 
> > > amdgpu_dm_connector *connector)
> > >       if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
> > >           debugfs_create_file_unsafe("psr_capability", 0444, dir, 
> > > connector, &psr_capability_fops);
> > >           debugfs_create_file_unsafe("psr_state", 0444, dir, 
> > > connector, &psr_fops);
> > > +        debugfs_create_file_unsafe("psr_residency", 0444, dir, 
> > > connector, &psr_residency_fops);
> > >           debugfs_create_file("amdgpu_current_backlight_pwm", 0444, 
> > > dir, connector,
> > >                       &current_backlight_fops);
> > >           debugfs_create_file("amdgpu_target_backlight_pwm", 0444, 
> > > dir, connector,