| Message ID | 1407172445-5730-1-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 04, 2014 at 10:44:04PM +0530, Vandana Kannan wrote: > CZ clock is related to data flow from memory to display plane. This is > required for comparison with CD clock before programming PFI credits. > > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ > 3 files changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 174a294..baeb56f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); > int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); > int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); > > +int valleyview_get_cz_clock_speed(struct drm_device *dev); > + > #define FORCEWAKE_RENDER (1 << 0) > #define FORCEWAKE_MEDIA (1 << 1) > #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index fe5c276..1b8f095 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -601,6 +601,7 @@ enum punit_power_well { > #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) > #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 > #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) > +#define CCK_CZ_CONTROL 0x62 Please move the reg define to just above DISPLAY_CLOCK_CONTROL to keep things in neat order. Also it might be a good idea to rename the DISPLAY_TRUNK_* and DISPLAY_FREQUENCY_* bits to CCK_... instead of DISPLAY_... to make it clear they apply to all CCK clock control registers. > > /** > * DOC: DPIO > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 99eb7ca..56a8090 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) > return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > } > > +int valleyview_get_cz_clock_speed(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + int vco = valleyview_get_vco(dev_priv); > + u32 val; > + int divider; > + > + mutex_lock(&dev_priv->dpio_lock); > + val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL); > + mutex_unlock(&dev_priv->dpio_lock); > + > + divider = val & DISPLAY_FREQUENCY_VALUES; > + > + WARN((val & DISPLAY_FREQUENCY_STATUS) != > + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), > + "czclk change in progress\n"); > + > + return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > +} We might want to refactor this a bit. Eg.: static int valleyview_get_cck_clock_speed(u32 reg, const char *name) { ... do the cck read etc. from CCK clock control register 'reg' } static int valleyview_get_cz_clock_speed() { return valleyview_get_cck_clock_speed(CCK_CZ_CLOCK_CONTROL, "czclk"); } static int valleyview_get_display_clock_speed() { ... return valleyview_get_cck_clock_speed(CCK_DISPLAY_CLOCK_CONTROL, "cdclk"); } > + > static int i945_get_display_clock_speed(struct drm_device *dev) > { > return 400000; > -- > 2.0.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Aug-05-2014 6:39 PM, Ville Syrjälä wrote: > On Mon, Aug 04, 2014 at 10:44:04PM +0530, Vandana Kannan wrote: >> CZ clock is related to data flow from memory to display plane. This is >> required for comparison with CD clock before programming PFI credits. >> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ >> 3 files changed, 23 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 174a294..baeb56f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); >> int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); >> int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); >> >> +int valleyview_get_cz_clock_speed(struct drm_device *dev); >> + >> #define FORCEWAKE_RENDER (1 << 0) >> #define FORCEWAKE_MEDIA (1 << 1) >> #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index fe5c276..1b8f095 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -601,6 +601,7 @@ enum punit_power_well { >> #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) >> #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 >> #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) >> +#define CCK_CZ_CONTROL 0x62 > > Please move the reg define to just above DISPLAY_CLOCK_CONTROL to keep > things in neat order. > Ok, will make this change.. > Also it might be a good idea to rename the DISPLAY_TRUNK_* and > DISPLAY_FREQUENCY_* bits to CCK_... instead of DISPLAY_... to make > it clear they apply to all CCK clock control registers. > Will submit this renaming part as a separate patch.. >> >> /** >> * DOC: DPIO >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 99eb7ca..56a8090 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) >> return DIV_ROUND_CLOSEST(vco << 1, divider + 1); >> } >> >> +int valleyview_get_cz_clock_speed(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + int vco = valleyview_get_vco(dev_priv); >> + u32 val; >> + int divider; >> + >> + mutex_lock(&dev_priv->dpio_lock); >> + val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL); >> + mutex_unlock(&dev_priv->dpio_lock); >> + >> + divider = val & DISPLAY_FREQUENCY_VALUES; >> + >> + WARN((val & DISPLAY_FREQUENCY_STATUS) != >> + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), >> + "czclk change in progress\n"); >> + >> + return DIV_ROUND_CLOSEST(vco << 1, divider + 1); >> +} > > We might want to refactor this a bit. Eg.: > > static int valleyview_get_cck_clock_speed(u32 reg, const char *name) > { > ... do the cck read etc. from CCK clock control register 'reg' > } > > static int valleyview_get_cz_clock_speed() > { > return valleyview_get_cck_clock_speed(CCK_CZ_CLOCK_CONTROL, "czclk"); > } > > static int valleyview_get_display_clock_speed() > { > ... > return valleyview_get_cck_clock_speed(CCK_DISPLAY_CLOCK_CONTROL, "cdclk"); > } > As part of the above refactoring, how about having an enum to specify CDCLK or CZCLK instead of char *? - Vandana >> + >> static int i945_get_display_clock_speed(struct drm_device *dev) >> { >> return 400000; >> -- >> 2.0.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Tue, Aug 05, 2014 at 08:57:07PM +0530, Vandana Kannan wrote: > On Aug-05-2014 6:39 PM, Ville Syrjälä wrote: > > On Mon, Aug 04, 2014 at 10:44:04PM +0530, Vandana Kannan wrote: > >> CZ clock is related to data flow from memory to display plane. This is > >> required for comparison with CD clock before programming PFI credits. > >> > >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ > >> drivers/gpu/drm/i915/i915_reg.h | 1 + > >> drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ > >> 3 files changed, 23 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index 174a294..baeb56f 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); > >> int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); > >> int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); > >> > >> +int valleyview_get_cz_clock_speed(struct drm_device *dev); > >> + > >> #define FORCEWAKE_RENDER (1 << 0) > >> #define FORCEWAKE_MEDIA (1 << 1) > >> #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >> index fe5c276..1b8f095 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -601,6 +601,7 @@ enum punit_power_well { > >> #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) > >> #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 > >> #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) > >> +#define CCK_CZ_CONTROL 0x62 > > > > Please move the reg define to just above DISPLAY_CLOCK_CONTROL to keep > > things in neat order. > > > Ok, will make this change.. > > > Also it might be a good idea to rename the DISPLAY_TRUNK_* and > > DISPLAY_FREQUENCY_* bits to CCK_... instead of DISPLAY_... to make > > it clear they apply to all CCK clock control registers. > > > Will submit this renaming part as a separate patch.. > >> > >> /** > >> * DOC: DPIO > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 99eb7ca..56a8090 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) > >> return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > >> } > >> > >> +int valleyview_get_cz_clock_speed(struct drm_device *dev) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + int vco = valleyview_get_vco(dev_priv); > >> + u32 val; > >> + int divider; > >> + > >> + mutex_lock(&dev_priv->dpio_lock); > >> + val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL); > >> + mutex_unlock(&dev_priv->dpio_lock); > >> + > >> + divider = val & DISPLAY_FREQUENCY_VALUES; > >> + > >> + WARN((val & DISPLAY_FREQUENCY_STATUS) != > >> + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), > >> + "czclk change in progress\n"); > >> + > >> + return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > >> +} > > > > We might want to refactor this a bit. Eg.: > > > > static int valleyview_get_cck_clock_speed(u32 reg, const char *name) > > { > > ... do the cck read etc. from CCK clock control register 'reg' > > } > > > > static int valleyview_get_cz_clock_speed() > > { > > return valleyview_get_cck_clock_speed(CCK_CZ_CLOCK_CONTROL, "czclk"); > > } > > > > static int valleyview_get_display_clock_speed() > > { > > ... > > return valleyview_get_cck_clock_speed(CCK_DISPLAY_CLOCK_CONTROL, "cdclk"); > > } > > > As part of the above refactoring, how about having an enum to specify > CDCLK or CZCLK instead of char *? We had such a thing in intel_i2c.c until I killed it here: drm/i915: Kill duplicated cdclk readout code from i2c Personally I don't see much point in having it since we'd just have these few users. But if you want to add it I don't mind too much either. > > - Vandana > >> + > >> static int i945_get_display_clock_speed(struct drm_device *dev) > >> { > >> return 400000; > >> -- > >> 2.0.1 > >> > >> _______________________________________________ > >> 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_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 174a294..baeb56f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); +int valleyview_get_cz_clock_speed(struct drm_device *dev); + #define FORCEWAKE_RENDER (1 << 0) #define FORCEWAKE_MEDIA (1 << 1) #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index fe5c276..1b8f095 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -601,6 +601,7 @@ enum punit_power_well { #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) +#define CCK_CZ_CONTROL 0x62 /** * DOC: DPIO diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 99eb7ca..56a8090 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) return DIV_ROUND_CLOSEST(vco << 1, divider + 1); } +int valleyview_get_cz_clock_speed(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + int vco = valleyview_get_vco(dev_priv); + u32 val; + int divider; + + mutex_lock(&dev_priv->dpio_lock); + val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL); + mutex_unlock(&dev_priv->dpio_lock); + + divider = val & DISPLAY_FREQUENCY_VALUES; + + WARN((val & DISPLAY_FREQUENCY_STATUS) != + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), + "czclk change in progress\n"); + + return DIV_ROUND_CLOSEST(vco << 1, divider + 1); +} + static int i945_get_display_clock_speed(struct drm_device *dev) { return 400000;
CZ clock is related to data flow from memory to display plane. This is required for comparison with CD clock before programming PFI credits. Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ 3 files changed, 23 insertions(+)