Re: [Intel-gfx] [PATCH v4 2/2] drm/i915/display: Support Multiple Transcoders' PSR status on debugfs

["Mun, Gwan-gyeong" <gwan-gyeong.mun@intel.com>]

On Wed, 2020-11-18 at 13:11 +0200, Jani Nikula wrote:
> On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> wrote:
> > In order to support the PSR state of each transcoder, it adds
> > i915_psr_status to sub-directory of each transcoder.
> > 
> > v2: Change using of Symbolic permissions 'S_IRUGO' to using of
> > octal
> >     permissions '0444'
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 23
> > +++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 8402e6ac9f76..37805615a221 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -2093,6 +2093,23 @@ static int
> > i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  
> > +static int i915_psr_status_show(struct seq_file *m, void *data)
> > +{
> > +	struct drm_connector *connector = m->private;
> > +	struct intel_dp *intel_dp =
> > +		intel_attached_dp(to_intel_connector(connector));
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > +	if (connector->status != connector_status_connected)
> > +		return -ENODEV;
> > +
> > +	if (!HAS_PSR(dev_priv))
> > +		return -ENODEV;
> > +
> > +	return intel_psr_status(m, intel_dp);
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status);
> > +
> >  #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n")
> > : \
> >  				seq_puts(m, "LPSP: incapable\n"))
> >  
> > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct
> > drm_connector *connector)
> >  				    connector,
> > &i915_psr_sink_status_fops);
> >  	}
> >  
> > +	if (INTEL_GEN(dev_priv) >= 12 &&
> 
> I'd add this for all generations to unify the debugfs, and eventually
> phase out the non connector specific debugfs file.
> 
> And I'd add HAS_PSR() check here to not create the file if it's not
> possible instead of having the check in i915_psr_status_show().
> 
Hi Jani,
Thank you for checking the patch.
I'll update the code as per your recommendations.
> BR,
> Jani.
> 
> > +	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +		debugfs_create_file("i915_psr_status", 0444, root,
> > +				    connector, &i915_psr_status_fops);
> > +	}
> > +
> >  	if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort
> > ||
> >  	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> >  	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx