| Message ID | 1442599791-2330-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2015-09-18 at 23:39 +0530, Sagar Arun Kamble wrote: > From: Akash Goel <akash.goel@intel.com> > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > Signed-off-by: Akash Goel <akash.goel@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 67bf205..6b1998c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2802,8 +2802,11 @@ enum skl_disp_power_wells { > > #define INTERVAL_1_28_US(us) (((us) * 100) >> 7) > #define INTERVAL_1_33_US(us) (((us) * 3) >> 2) > +#define INTERVAL_0_833_US(us) (((us) * 6) / 5) > #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \ > - INTERVAL_1_33_US(us) : \ > + (IS_BROXTON(dev_priv) ? \ > + INTERVAL_0_833_US(us) : \ > + INTERVAL_1_33_US(us)) : \ > INTERVAL_1_28_US(us)) Note that neither of the GEN9 values are mentioned in Bspec, so a request for update should be filed there. I haven't found any other place that would document this either, could you point me to it? --Imre
On pe, 2015-09-18 at 23:39 +0530, Sagar Arun Kamble wrote: > From: Akash Goel <akash.goel@intel.com> > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > Signed-off-by: Akash Goel <akash.goel@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> The comment about units in gen6_set_rps_thresholds() is outdated, so you could update that while at it. In any case this looks ok, so: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 67bf205..6b1998c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2802,8 +2802,11 @@ enum skl_disp_power_wells { > > #define INTERVAL_1_28_US(us) (((us) * 100) >> 7) > #define INTERVAL_1_33_US(us) (((us) * 3) >> 2) > +#define INTERVAL_0_833_US(us) (((us) * 6) / 5) > #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \ > - INTERVAL_1_33_US(us) : \ > + (IS_BROXTON(dev_priv) ? \ > + INTERVAL_0_833_US(us) : \ > + INTERVAL_1_33_US(us)) : \ > INTERVAL_1_28_US(us)) > > /*
On Tue, Oct 06, 2015 at 09:16:28PM +0300, Imre Deak wrote: > On pe, 2015-09-18 at 23:39 +0530, Sagar Arun Kamble wrote: > > From: Akash Goel <akash.goel@intel.com> > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > > Signed-off-by: Akash Goel <akash.goel@intel.com> > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > The comment about units in gen6_set_rps_thresholds() is outdated, so you > could update that while at it. In any case this looks ok, so: > Reviewed-by: Imre Deak <imre.deak@intel.com> In your previous review you noticed that Bspec is still outdated. Has that been fixed meanwhile? -Daniel > > > --- > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 67bf205..6b1998c 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -2802,8 +2802,11 @@ enum skl_disp_power_wells { > > > > #define INTERVAL_1_28_US(us) (((us) * 100) >> 7) > > #define INTERVAL_1_33_US(us) (((us) * 3) >> 2) > > +#define INTERVAL_0_833_US(us) (((us) * 6) / 5) > > #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \ > > - INTERVAL_1_33_US(us) : \ > > + (IS_BROXTON(dev_priv) ? \ > > + INTERVAL_0_833_US(us) : \ > > + INTERVAL_1_33_US(us)) : \ > > INTERVAL_1_28_US(us)) > > > > /* > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On ke, 2015-10-07 at 15:29 +0200, Daniel Vetter wrote: > On Tue, Oct 06, 2015 at 09:16:28PM +0300, Imre Deak wrote: > > On pe, 2015-09-18 at 23:39 +0530, Sagar Arun Kamble wrote: > > > From: Akash Goel <akash.goel@intel.com> > > > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > > > Signed-off-by: Akash Goel <akash.goel@intel.com> > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > The comment about units in gen6_set_rps_thresholds() is outdated, so you > > could update that while at it. In any case this looks ok, so: > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > In your previous review you noticed that Bspec is still outdated. Has that > been fixed meanwhile? Yep, Sagar pointed to a page in BSpec where the relevant clocks are defined ("Timestamp bases") and he also filed a change request in Bspec now to update the register description itself accordingly. Should've mentioned this when adding my r-b. --Imre > -Daniel > > > > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 67bf205..6b1998c 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -2802,8 +2802,11 @@ enum skl_disp_power_wells { > > > > > > #define INTERVAL_1_28_US(us) (((us) * 100) >> 7) > > > #define INTERVAL_1_33_US(us) (((us) * 3) >> 2) > > > +#define INTERVAL_0_833_US(us) (((us) * 6) / 5) > > > #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \ > > > - INTERVAL_1_33_US(us) : \ > > > + (IS_BROXTON(dev_priv) ? \ > > > + INTERVAL_0_833_US(us) : \ > > > + INTERVAL_1_33_US(us)) : \ > > > INTERVAL_1_28_US(us)) > > > > > > /* > > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, Oct 07, 2015 at 04:35:51PM +0300, Imre Deak wrote: > On ke, 2015-10-07 at 15:29 +0200, Daniel Vetter wrote: > > On Tue, Oct 06, 2015 at 09:16:28PM +0300, Imre Deak wrote: > > > On pe, 2015-09-18 at 23:39 +0530, Sagar Arun Kamble wrote: > > > > From: Akash Goel <akash.goel@intel.com> > > > > > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > > > > Signed-off-by: Akash Goel <akash.goel@intel.com> > > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > > > The comment about units in gen6_set_rps_thresholds() is outdated, so you > > > could update that while at it. In any case this looks ok, so: > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > In your previous review you noticed that Bspec is still outdated. Has that > > been fixed meanwhile? > > Yep, Sagar pointed to a page in BSpec where the relevant clocks are > defined ("Timestamp bases") and he also filed a change request in Bspec > now to update the register description itself accordingly. Should've > mentioned this when adding my r-b. Awesome. Added a note about this and applied the patch. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 67bf205..6b1998c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2802,8 +2802,11 @@ enum skl_disp_power_wells { #define INTERVAL_1_28_US(us) (((us) * 100) >> 7) #define INTERVAL_1_33_US(us) (((us) * 3) >> 2) +#define INTERVAL_0_833_US(us) (((us) * 6) / 5) #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \ - INTERVAL_1_33_US(us) : \ + (IS_BROXTON(dev_priv) ? \ + INTERVAL_0_833_US(us) : \ + INTERVAL_1_33_US(us)) : \ INTERVAL_1_28_US(us)) /*