| Message ID | 1413910688-1951-1-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Otherwise, a simple "cat" to the debugfs file can make the machine use > much more power than needed, and prevent it from runtime suspending. > > Related commit: > > commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 > Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Date: Tue Oct 7 17:21:26 2014 +0300 > drm/i915: Build workaround list in ring initialization > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Arun Siluvery <arun.siluvery@linux.intel.com> > Testcase: igt/pm_rpm/debugfs-read > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> tbh I'm not even sure we want to do the manual forcewake get here - I915_READ will do it for us, and this is a debug interface. So no one should care about perf. Mika, is that right? If so I'd like to merge the inverse patch which drops the fw_get. -Daniel > --- > drivers/gpu/drm/i915/i915_debugfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 9600285..36a4baa 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) > addr, value, mask, read, ok ? "OK" : "FAIL"); > } > > + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); > intel_runtime_pm_put(dev_priv); > mutex_unlock(&dev->struct_mutex); > > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote: > On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Otherwise, a simple "cat" to the debugfs file can make the machine use > > much more power than needed, and prevent it from runtime suspending. > > > > Related commit: > > > > commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 > > Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Date: Tue Oct 7 17:21:26 2014 +0300 > > drm/i915: Build workaround list in ring initialization > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Arun Siluvery <arun.siluvery@linux.intel.com> > > Testcase: igt/pm_rpm/debugfs-read > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > tbh I'm not even sure we want to do the manual forcewake get here - > I915_READ will do it for us, and this is a debug interface. So no one > should care about perf. Mika, is that right? If so I'd like to merge the > inverse patch which drops the fw_get. Don't we still need the idle msg disable+poll CSPWRFSM trick here on gen8? That also needs forcewake around it. > -Daniel > > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > index 9600285..36a4baa 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) > > addr, value, mask, read, ok ? "OK" : "FAIL"); > > } > > > > + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); > > intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > -- > > 2.1.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 22/10/2014 08:35, Ville Syrjälä wrote: > On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote: >> On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: >>> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> >>> Otherwise, a simple "cat" to the debugfs file can make the machine use >>> much more power than needed, and prevent it from runtime suspending. >>> >>> Related commit: >>> >>> commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 >>> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>> Date: Tue Oct 7 17:21:26 2014 +0300 >>> drm/i915: Build workaround list in ring initialization >>> >>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>> Cc: Arun Siluvery <arun.siluvery@linux.intel.com> >>> Testcase: igt/pm_rpm/debugfs-read >>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> tbh I'm not even sure we want to do the manual forcewake get here - >> I915_READ will do it for us, and this is a debug interface. So no one >> should care about perf. Mika, is that right? If so I'd like to merge the >> inverse patch which drops the fw_get. > > Don't we still need the idle msg disable+poll CSPWRFSM trick here on > gen8? That also needs forcewake around it. > I had a chat with Mika on this yesterday and he seem to agree that forcewake is probably not required here. I couldn't send the patch yesterday but as per Ville's comments looks like we need forcewake here? regards Arun >> -Daniel >> >>> --- >>> drivers/gpu/drm/i915/i915_debugfs.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >>> index 9600285..36a4baa 100644 >>> --- a/drivers/gpu/drm/i915/i915_debugfs.c >>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >>> @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) >>> addr, value, mask, read, ok ? "OK" : "FAIL"); >>> } >>> >>> + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); >>> intel_runtime_pm_put(dev_priv); >>> mutex_unlock(&dev->struct_mutex); >>> >>> -- >>> 2.1.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
2014-10-22 6:56 GMT-02:00 Siluvery, Arun <arun.siluvery@linux.intel.com>: > On 22/10/2014 08:35, Ville Syrjälä wrote: >> >> On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote: >>> >>> On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: >>>> >>>> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >>>> >>>> Otherwise, a simple "cat" to the debugfs file can make the machine use >>>> much more power than needed, and prevent it from runtime suspending. >>>> >>>> Related commit: >>>> >>>> commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 >>>> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>>> Date: Tue Oct 7 17:21:26 2014 +0300 >>>> drm/i915: Build workaround list in ring initialization >>>> >>>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>>> Cc: Arun Siluvery <arun.siluvery@linux.intel.com> >>>> Testcase: igt/pm_rpm/debugfs-read >>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> >>> >>> tbh I'm not even sure we want to do the manual forcewake get here - >>> I915_READ will do it for us, and this is a debug interface. So no one >>> should care about perf. Mika, is that right? If so I'd like to merge the >>> inverse patch which drops the fw_get. >> >> >> Don't we still need the idle msg disable+poll CSPWRFSM trick here on >> gen8? That also needs forcewake around it. >> > > I had a chat with Mika on this yesterday and he seem to agree that forcewake > is probably not required here. I couldn't send the patch yesterday but as > per Ville's comments looks like we need forcewake here? Looks like Daniel removed the forcewake get from the original patch. > > regards > Arun > > >>> -Daniel >>> >>>> --- >>>> drivers/gpu/drm/i915/i915_debugfs.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c >>>> b/drivers/gpu/drm/i915/i915_debugfs.c >>>> index 9600285..36a4baa 100644 >>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c >>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >>>> @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, >>>> void *unused) >>>> addr, value, mask, read, ok ? "OK" : "FAIL"); >>>> } >>>> >>>> + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); >>>> intel_runtime_pm_put(dev_priv); >>>> mutex_unlock(&dev->struct_mutex); >>>> >>>> -- >>>> 2.1.1 >>>> >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> >>> >>> -- >>> Daniel Vetter >>> Software Engineer, Intel Corporation >>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9600285..36a4baa 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) addr, value, mask, read, ok ? "OK" : "FAIL"); } + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex);