[Intel-gfx] [PATCH 01/19] drm/i915: WARN if !HAS_PC8 when enabling/disabling PC8

[Paulo Zanoni]

2013/11/29 Rodrigo Vivi <rodrigo.vivi at gmail.com>:
> I think it should be a return instead a WARN.
> Myabe I'm just sleeping yet, but I think the delayed work will execute this even for non HAS_PC8 and warn will always ring on non HSW.
> But even if I'm sleeping, since it really touch hsw registers wouldn't be safer a return instead of a warn anyway?

The refcount should never reach zero on platforms that don't have PC8,
so this should never happen. Also, functions that lead to this
function also have early returns, so we really shouldn't hit this
thing at all.

The early return will hide the problem, while the WARN will tell me my
design is wrong and I need to fix bugs :)

>
>
> On Thu, Nov 21, 2013 at 01:47:15PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> We already have some checks and shouldn't be reaching these places on
>> !HAS_PC8 platforms, but add a WARN,  just in case.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e85d838..5566de5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6623,6 +6623,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>>       struct drm_device *dev = dev_priv->dev;
>>       uint32_t val;
>>
>> +     WARN_ON(!HAS_PC8(dev));
>> +
>>       if (dev_priv->pc8.enabled)
>>               return;
>>
>> @@ -6668,6 +6670,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
>>       if (dev_priv->pc8.disable_count != 1)
>>               return;
>>
>> +     WARN_ON(!HAS_PC8(dev));
>> +
>>       cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
>>       if (!dev_priv->pc8.enabled)
>>               return;
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni