Re: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

["S, Deepak" <deepak.s@intel.com>]

On 1/30/2014 1:00 AM, Daniel Vetter wrote:
> On Wed, Jan 29, 2014 at 05:59:23PM +0200, Ville Syrjälä wrote:
>> On Mon, Jan 27, 2014 at 09:35:05PM +0530, deepak.s@intel.com wrote:
>>> From: Deepak S <deepak.s@intel.com>
>>>
>>> When current delay is already at max delay, Let's disable the PM UP
>>> THRESHOLD INTRRUPTS, so that we will not get further interrupts until
>>> current delay is less than max delay, Also request for the PM DOWN
>>> THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
>>> viceversa for PM DOWN THRESHOLD INTRRUPTS.
>>>
>>> v2: Use bool variables (Daniel)
>>>
>>> v3: Fix Interrupt masking bit (Deepak)
>>>
>>> v4: Use existing symbolic constants in i915_reg.h (Daniel)
>>>
>>> v5: Add pm interrupt mask after new_delay calculation (Ville)
>>>
>>> Signed-off-by: Deepak S <deepak.s@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h |  3 +++
>>>   drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_pm.c |  3 +++
>>>   3 files changed, 45 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 56c720b..f19de66 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
>>>   	u8 rp0_delay;
>>>   	u8 hw_max;
>>>
>>> +	bool rp_up_masked;
>>> +	bool rp_down_masked;
>>> +
>>>   	int last_adj;
>>>   	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>> index 01a8686..69a5214 100644
>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> @@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
>>>   	i915_queue_hangcheck(dev);
>>>   }
>>>
>>> +static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
>>> +						u32 pm_iir, int *new_delay)
>>
>> Just a minor nit here. We don't modify new_delay in this function, so
>> passing by value would be better.
>
> I've fixed this up and merged the patch. I've also polished the whitespace
> a bit, please run patches through scripts/checkpatch.pl before submitting.
> I usually don't all go whitespace-nazi about this, but generally the
> suggestions result in more uniform and hence readable sources.
>
>> Otherwise the patch looks good to me. So if you change that, you can
>> add:
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Thanks, Daniel

Thanks Daniel.