diff mbox series

drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit

Message ID 20180811005031.8888-1-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit | expand

Commit Message

Dhinakaran Pandiyan Aug. 11, 2018, 12:50 a.m. UTC
We print the last attempted entry and last exit timestamps only when
IRQ debug is requested. This check was missed when new debug flags were
added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
runtime through debugfs, v6")

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

kernel test robot Aug. 12, 2018, 8 a.m. UTC | #1
Hi Dhinakaran,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.18-rc8 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dhinakaran-Pandiyan/drm-i915-psr-Add-missing-check-for-I915_PSR_DEBUG_IRQ-bit/20180812-143531
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x018-201832 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_debugfs.c: In function 'i915_edp_psr_status':
>> drivers/gpu//drm/i915/i915_debugfs.c:2738:39: error: 'I915_PSR_DEBUG_IRQ' undeclared (first use in this function); did you mean 'EDP_PSR_DEBUG'?
     if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
                                          ^~~~~~~~~~~~~~~~~~
                                          EDP_PSR_DEBUG
   drivers/gpu//drm/i915/i915_debugfs.c:2738:39: note: each undeclared identifier is reported only once for each function it appears in

vim +2738 drivers/gpu//drm/i915/i915_debugfs.c

  2692	
  2693	static int i915_edp_psr_status(struct seq_file *m, void *data)
  2694	{
  2695		struct drm_i915_private *dev_priv = node_to_i915(m->private);
  2696		u32 psrperf = 0;
  2697		bool enabled = false;
  2698		bool sink_support;
  2699	
  2700		if (!HAS_PSR(dev_priv))
  2701			return -ENODEV;
  2702	
  2703		sink_support = dev_priv->psr.sink_support;
  2704		seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
  2705		if (!sink_support)
  2706			return 0;
  2707	
  2708		intel_runtime_pm_get(dev_priv);
  2709	
  2710		mutex_lock(&dev_priv->psr.lock);
  2711		seq_printf(m, "Enabled: %s\n", yesno((bool)dev_priv->psr.enabled));
  2712		seq_printf(m, "Busy frontbuffer bits: 0x%03x\n",
  2713			   dev_priv->psr.busy_frontbuffer_bits);
  2714	
  2715		if (dev_priv->psr.psr2_enabled)
  2716			enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
  2717		else
  2718			enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
  2719	
  2720		seq_printf(m, "Main link in standby mode: %s\n",
  2721			   yesno(dev_priv->psr.link_standby));
  2722	
  2723		seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
  2724	
  2725		/*
  2726		 * SKL+ Perf counter is reset to 0 everytime DC state is entered
  2727		 */
  2728		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
  2729			psrperf = I915_READ(EDP_PSR_PERF_CNT) &
  2730				EDP_PSR_PERF_CNT_MASK;
  2731	
  2732			seq_printf(m, "Performance_Counter: %u\n", psrperf);
  2733		}
  2734	
  2735		psr_source_status(dev_priv, m);
  2736		mutex_unlock(&dev_priv->psr.lock);
  2737	
> 2738		if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
  2739			seq_printf(m, "Last attempted entry at: %lld\n",
  2740				   dev_priv->psr.last_entry_attempt);
  2741			seq_printf(m, "Last exit at: %lld\n",
  2742				   dev_priv->psr.last_exit);
  2743		}
  2744	
  2745		intel_runtime_pm_put(dev_priv);
  2746		return 0;
  2747	}
  2748	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Aug. 12, 2018, 9:26 a.m. UTC | #2
Hi Dhinakaran,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.18-rc8 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dhinakaran-Pandiyan/drm-i915-psr-Add-missing-check-for-I915_PSR_DEBUG_IRQ-bit/20180812-143531
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-s1-08121632 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from include/linux/debugfs.h:15,
                    from drivers/gpu/drm/i915/i915_debugfs.c:29:
   drivers/gpu/drm/i915/i915_debugfs.c: In function 'i915_edp_psr_status':
   drivers/gpu/drm/i915/i915_debugfs.c:2738:39: error: 'I915_PSR_DEBUG_IRQ' undeclared (first use in this function)
     if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
                                          ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/i915/i915_debugfs.c:2738:2: note: in expansion of macro 'if'
     if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
     ^~
   drivers/gpu/drm/i915/i915_debugfs.c:2738:39: note: each undeclared identifier is reported only once for each function it appears in
     if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
                                          ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/i915/i915_debugfs.c:2738:2: note: in expansion of macro 'if'
     if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
     ^~

vim +/if +2738 drivers/gpu/drm/i915/i915_debugfs.c

  2692	
  2693	static int i915_edp_psr_status(struct seq_file *m, void *data)
  2694	{
  2695		struct drm_i915_private *dev_priv = node_to_i915(m->private);
  2696		u32 psrperf = 0;
  2697		bool enabled = false;
  2698		bool sink_support;
  2699	
  2700		if (!HAS_PSR(dev_priv))
  2701			return -ENODEV;
  2702	
  2703		sink_support = dev_priv->psr.sink_support;
  2704		seq_printf(m, "Sink_Support: %s\n", yesno(sink_support));
  2705		if (!sink_support)
  2706			return 0;
  2707	
  2708		intel_runtime_pm_get(dev_priv);
  2709	
  2710		mutex_lock(&dev_priv->psr.lock);
  2711		seq_printf(m, "Enabled: %s\n", yesno((bool)dev_priv->psr.enabled));
  2712		seq_printf(m, "Busy frontbuffer bits: 0x%03x\n",
  2713			   dev_priv->psr.busy_frontbuffer_bits);
  2714	
  2715		if (dev_priv->psr.psr2_enabled)
  2716			enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
  2717		else
  2718			enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
  2719	
  2720		seq_printf(m, "Main link in standby mode: %s\n",
  2721			   yesno(dev_priv->psr.link_standby));
  2722	
  2723		seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
  2724	
  2725		/*
  2726		 * SKL+ Perf counter is reset to 0 everytime DC state is entered
  2727		 */
  2728		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
  2729			psrperf = I915_READ(EDP_PSR_PERF_CNT) &
  2730				EDP_PSR_PERF_CNT_MASK;
  2731	
  2732			seq_printf(m, "Performance_Counter: %u\n", psrperf);
  2733		}
  2734	
  2735		psr_source_status(dev_priv, m);
  2736		mutex_unlock(&dev_priv->psr.lock);
  2737	
> 2738		if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
  2739			seq_printf(m, "Last attempted entry at: %lld\n",
  2740				   dev_priv->psr.last_entry_attempt);
  2741			seq_printf(m, "Last exit at: %lld\n",
  2742				   dev_priv->psr.last_exit);
  2743		}
  2744	
  2745		intel_runtime_pm_put(dev_priv);
  2746		return 0;
  2747	}
  2748	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Maarten Lankhorst Aug. 13, 2018, 1:47 p.m. UTC | #3
Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> We print the last attempted entry and last exit timestamps only when
> IRQ debug is requested. This check was missed when new debug flags were
> added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> runtime through debugfs, v6")
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 26b7e5276b15..374b550d9a4f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>  	psr_source_status(dev_priv, m);
>  	mutex_unlock(&dev_priv->psr.lock);
>  
> -	if (READ_ONCE(dev_priv->psr.debug)) {
> +	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
>  		seq_printf(m, "Last attempted entry at: %lld\n",
>  			   dev_priv->psr.last_entry_attempt);
>  		seq_printf(m, "Last exit at: %lld\n",

Oops indeed.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Rodrigo Vivi Aug. 13, 2018, 4:47 p.m. UTC | #4
On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
> Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> > We print the last attempted entry and last exit timestamps only when
> > IRQ debug is requested. This check was missed when new debug flags were
> > added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> > runtime through debugfs, v6")
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 26b7e5276b15..374b550d9a4f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  	psr_source_status(dev_priv, m);
> >  	mutex_unlock(&dev_priv->psr.lock);
> >  
> > -	if (READ_ONCE(dev_priv->psr.debug)) {
> > +	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
> >  		seq_printf(m, "Last attempted entry at: %lld\n",
> >  			   dev_priv->psr.last_entry_attempt);
> >  		seq_printf(m, "Last exit at: %lld\n",
> 
> Oops indeed.
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

before pushing to dinq please check the compilation there..
kbuild bot raised an issue...

so apparently we will need a backmerge before pushing this...

> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Aug. 13, 2018, 6:15 p.m. UTC | #5
On Mon, 2018-08-13 at 09:47 -0700, Rodrigo Vivi wrote:
> On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
> > Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> > > We print the last attempted entry and last exit timestamps only
> > > when
> > > IRQ debug is requested. This check was missed when new debug
> > > flags were
> > > added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> > > runtime through debugfs, v6")
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> > > >
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 26b7e5276b15..374b550d9a4f 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct
> > > seq_file *m, void *data)
> > >  	psr_source_status(dev_priv, m);
> > >  	mutex_unlock(&dev_priv->psr.lock);
> > >  
> > > -	if (READ_ONCE(dev_priv->psr.debug)) {
> > > +	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ)
> > > {
> > >  		seq_printf(m, "Last attempted entry at: %lld\n",
> > >  			   dev_priv->psr.last_entry_attempt);
> > >  		seq_printf(m, "Last exit at: %lld\n",
> > 
> > Oops indeed.
> > 
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> before pushing to dinq please check the compilation there..
> kbuild bot raised an issue...
> 
> so apparently we will need a backmerge before pushing this...

The failures are on 

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.18-rc8 next-20180810]

Is a back-merge expected to fix that? and who does that back-merge?

-DK
> 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst Aug. 14, 2018, 9:26 a.m. UTC | #6
Op 13-08-18 om 20:15 schreef Pandiyan, Dhinakaran:
> On Mon, 2018-08-13 at 09:47 -0700, Rodrigo Vivi wrote:
>> On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
>>> Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
>>>> We print the last attempted entry and last exit timestamps only
>>>> when
>>>> IRQ debug is requested. This check was missed when new debug
>>>> flags were
>>>> added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
>>>> runtime through debugfs, v6")
>>>>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
>>>> ---
>>>>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> index 26b7e5276b15..374b550d9a4f 100644
>>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct
>>>> seq_file *m, void *data)
>>>>  	psr_source_status(dev_priv, m);
>>>>  	mutex_unlock(&dev_priv->psr.lock);
>>>>  
>>>> -	if (READ_ONCE(dev_priv->psr.debug)) {
>>>> +	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ)
>>>> {
>>>>  		seq_printf(m, "Last attempted entry at: %lld\n",
>>>>  			   dev_priv->psr.last_entry_attempt);
>>>>  		seq_printf(m, "Last exit at: %lld\n",
>>> Oops indeed.
>>>
>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> before pushing to dinq please check the compilation there..
>> kbuild bot raised an issue...
>>
>> so apparently we will need a backmerge before pushing this...
> The failures are on 
>
> [auto build test ERROR on drm-intel/for-linux-next]
> [also build test ERROR on v4.18-rc8 next-20180810]
>
> Is a back-merge expected to fix that? and who does that back-merge?
Yes, this fix should have been pushed to drm-misc-next. So both branches need to be merged. :)
Rodrigo Vivi Aug. 15, 2018, 9:06 p.m. UTC | #7
On Tue, Aug 14, 2018 at 11:26:19AM +0200, Maarten Lankhorst wrote:
> Op 13-08-18 om 20:15 schreef Pandiyan, Dhinakaran:
> > On Mon, 2018-08-13 at 09:47 -0700, Rodrigo Vivi wrote:
> >> On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
> >>> Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> >>>> We print the last attempted entry and last exit timestamps only
> >>>> when
> >>>> IRQ debug is requested. This check was missed when new debug
> >>>> flags were
> >>>> added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> >>>> runtime through debugfs, v6")
> >>>>
> >>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> >>>> ---
> >>>>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> >>>> b/drivers/gpu/drm/i915/i915_debugfs.c
> >>>> index 26b7e5276b15..374b550d9a4f 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >>>> @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct
> >>>> seq_file *m, void *data)
> >>>>  	psr_source_status(dev_priv, m);
> >>>>  	mutex_unlock(&dev_priv->psr.lock);
> >>>>  
> >>>> -	if (READ_ONCE(dev_priv->psr.debug)) {
> >>>> +	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ)
> >>>> {
> >>>>  		seq_printf(m, "Last attempted entry at: %lld\n",
> >>>>  			   dev_priv->psr.last_entry_attempt);
> >>>>  		seq_printf(m, "Last exit at: %lld\n",
> >>> Oops indeed.
> >>>
> >>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> before pushing to dinq please check the compilation there..
> >> kbuild bot raised an issue...
> >>
> >> so apparently we will need a backmerge before pushing this...
> > The failures are on 
> >
> > [auto build test ERROR on drm-intel/for-linux-next]
> > [also build test ERROR on v4.18-rc8 next-20180810]

I don't expect this patch on any of this, so let's just ignore it ;)

now I'm asking myself why exactly kbuild bot is trying to apply
patches targeting 4.20 on branches targeting 4.18 and 4.19...

?!  :/

> > Is a back-merge expected to fix that?

my concern was more about having this gap on dinq.

I checked and we are good to push this through dinq
no backmerge needed
so feel free to go ahead.

> and who does that back-merge?

maintainers per need bases

> Yes, this fix should have been pushed to drm-misc-next. So both branches need to be merged. :)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 26b7e5276b15..374b550d9a4f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2735,7 +2735,7 @@  static int i915_edp_psr_status(struct seq_file *m, void *data)
 	psr_source_status(dev_priv, m);
 	mutex_unlock(&dev_priv->psr.lock);
 
-	if (READ_ONCE(dev_priv->psr.debug)) {
+	if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
 		seq_printf(m, "Last attempted entry at: %lld\n",
 			   dev_priv->psr.last_entry_attempt);
 		seq_printf(m, "Last exit at: %lld\n",