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@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
