diff mbox

[4/9] drm/i915/bdw: Use centralized rc6 info print

Message ID 1390969547-1018-5-git-send-email-benjamin.widawsky@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Jan. 29, 2014, 4:25 a.m. UTC 
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Rodrigo Vivi Feb. 6, 2014, 1:42 p.m. UTC | #1 
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
<benjamin.widawsky@intel.com> wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 944b99c..6acb429 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct drm_device *dev)
>         /* 3: Enable RC6 */
>         if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
>                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
> -       DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
> +       intel_print_rc6_info(dev, rc6_mask);
>         I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -                       GEN6_RC_CTL_EI_MODE(1) |
> -                       rc6_mask);
> +                                   GEN6_RC_CTL_EI_MODE(1) |
> +                                   rc6_mask);
>
>         /* 4 Program defaults and thresholds for RPS*/
>         I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */
> --
> 1.8.5.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
deepak.s@intel.com Feb. 7, 2014, 5:46 a.m. UTC | #2 
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky
> <benjamin.widawsky@intel.com <mailto:benjamin.widawsky@intel.com>> wrote:
>
>     Signed-off-by: Ben Widawsky <ben@bwidawsk.net <mailto:ben@bwidawsk.net>>
>     ---
>       drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>       1 file changed, 3 insertions(+), 3 deletions(-)
>
>     diff --git a/drivers/gpu/drm/i915/intel_pm.c
>     b/drivers/gpu/drm/i915/intel_pm.c
>     index 944b99c..6acb429 100644
>     --- a/drivers/gpu/drm/i915/intel_pm.c
>     +++ b/drivers/gpu/drm/i915/intel_pm.c
>     @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct
>     drm_device *dev)
>              /* 3: Enable RC6 */
>              if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
>                      rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>     -       DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
>     "on" : "off");
>     +       intel_print_rc6_info(dev, rc6_mask);
>              I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>     -                       GEN6_RC_CTL_EI_MODE(1) |
>     -                       rc6_mask);
>     +                                   GEN6_RC_CTL_EI_MODE(1) |
>     +                                   rc6_mask);
>
>              /* 4 Program defaults and thresholds for RPS*/
>              I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request
>     500 MHz */
>     --
>     1.8.5.3
>
>     _______________________________________________
>     Intel-gfx mailing list
>     Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Reviewed-by: Deepak S <deepak.s@intel.com>
Daniel Vetter Feb. 11, 2014, 4:12 p.m. UTC | #3 
On Thu, Feb 06, 2014 at 11:42:39AM -0200, Rodrigo Vivi wrote:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
> <benjamin.widawsky@intel.com> wrote:
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 944b99c..6acb429 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct drm_device *dev)
> >         /* 3: Enable RC6 */
> >         if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
> >                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
> > -       DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
> > +       intel_print_rc6_info(dev, rc6_mask);
> >         I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> > -                       GEN6_RC_CTL_EI_MODE(1) |
> > -                       rc6_mask);
> > +                                   GEN6_RC_CTL_EI_MODE(1) |
> > +                                   rc6_mask);
> >
> >         /* 4 Program defaults and thresholds for RPS*/
> >         I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */
> > --
> > 1.8.5.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Merged up to this patch, but after that it gets confusing:
- I have two patches 5/9.
- 6/9 lacks review ...

Mailing-list grues ate something?
-Daniel
Ben Widawsky Feb. 14, 2014, 8:34 p.m. UTC | #4 
On Tue, Feb 11, 2014 at 05:12:17PM +0100, Daniel Vetter wrote:
> On Thu, Feb 06, 2014 at 11:42:39AM -0200, Rodrigo Vivi wrote:
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > 
> > On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
> > <benjamin.widawsky@intel.com> wrote:
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 944b99c..6acb429 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct drm_device *dev)
> > >         /* 3: Enable RC6 */
> > >         if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
> > >                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
> > > -       DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
> > > +       intel_print_rc6_info(dev, rc6_mask);
> > >         I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> > > -                       GEN6_RC_CTL_EI_MODE(1) |
> > > -                       rc6_mask);
> > > +                                   GEN6_RC_CTL_EI_MODE(1) |
> > > +                                   rc6_mask);
> > >
> > >         /* 4 Program defaults and thresholds for RPS*/
> > >         I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */
> > > --
> > > 1.8.5.3
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> Merged up to this patch, but after that it gets confusing:
> - I have two patches 5/9.

5/9 is good to go. Not sure how two got sent out. Rodrigo reviewed the
right one (the patches are the same, but the commit message changed). I
think I accidentally did two git format-patches in my staging area after
doing the rename.

> - 6/9 lacks review ...
> 

6/9 can be skipped for this series, though I think it's a reasonable
patch. Maybe Chris or Jesse has an opinion on 6/9?

> Mailing-list grues ate something?
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Chris Wilson Feb. 14, 2014, 8:41 p.m. UTC | #5 
On Fri, Feb 14, 2014 at 12:34:22PM -0800, Ben Widawsky wrote:
> On Tue, Feb 11, 2014 at 05:12:17PM +0100, Daniel Vetter wrote:
> > - 6/9 lacks review ...
> > 
> 
> 6/9 can be skipped for this series, though I think it's a reasonable
> patch. Maybe Chris or Jesse has an opinion on 6/9?

Yes, programming random values into the hardware is just bonghits. And
then people come along complaining how the idiotic programming guide is
sacrosanct and must be adherred to...

Otoh, changing the value of GEN6_RC_VIDEO_FREQ is worrying as we never
reprogram it nor have I come across any instructions as to what it does.
-Chris
Ben Widawsky Feb. 17, 2014, 7:41 p.m. UTC | #6 
On Fri, Feb 14, 2014 at 08:41:08PM +0000, Chris Wilson wrote:
> On Fri, Feb 14, 2014 at 12:34:22PM -0800, Ben Widawsky wrote:
> > On Tue, Feb 11, 2014 at 05:12:17PM +0100, Daniel Vetter wrote:
> > > - 6/9 lacks review ...
> > > 
> > 
> > 6/9 can be skipped for this series, though I think it's a reasonable
> > patch. Maybe Chris or Jesse has an opinion on 6/9?
> 
> Yes, programming random values into the hardware is just bonghits. And
> then people come along complaining how the idiotic programming guide is
> sacrosanct and must be adherred to...
> 
> Otoh, changing the value of GEN6_RC_VIDEO_FREQ is worrying as we never
> reprogram it nor have I come across any instructions as to what it does.
> -Chris

Can I take a _____by: if I drop GEN6_RC_VIDEO_FREQ?

> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 944b99c..6acb429 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3215,10 +3215,10 @@  static void gen8_enable_rps(struct drm_device *dev)
 	/* 3: Enable RC6 */
 	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
 		rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
-	DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
+	intel_print_rc6_info(dev, rc6_mask);
 	I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-			GEN6_RC_CTL_EI_MODE(1) |
-			rc6_mask);
+				    GEN6_RC_CTL_EI_MODE(1) |
+				    rc6_mask);
 
 	/* 4 Program defaults and thresholds for RPS*/
 	I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */