Re: [PATCH 07/19] drm/i915: add runtime put/get calls at the basic places

Paulo Zanoni
2013-11-25T20:55:54Z

2013/11/21 Chris Wilson <chris@chris-wilson.co.uk>:
> On Thu, Nov 21, 2013 at 01:47:21PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> If I add code to enable runtime PM on my Haswell machine, start a
>> desktop environment, then enable runtime PM, these functions will
>> complain that they're trying to read/write registers while the
>> graphics card is suspended.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++++++-----------
>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++
>> drivers/gpu/drm/i915/i915_irq.c | 6 ++++
>> 3 files changed, 46 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 40d9dcf..94c2a38 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1377,36 +1377,38 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> drm_i915_private_t *dev_priv = dev->dev_private;
>> pgoff_t page_offset;
>> unsigned long pfn;
>> - int ret = 0;
>> + int rc = 0, ret;
>
> Ugh. Just keep ret and don't add rc.

My idea was that "rc" would contain the return codes for the functions
we call, and "ret" would contain the value we want to return. With
this, at the end of the function we "switch (rc)" and then decide what
we'll assign to "ret". IMHO it's confusing to mix both: we'll "switch
(ret)" and then assign new values to "ret" inside the switch
statement. Also, we don't have to worry about mixing values like
EAGAIN and VM_FAULT_SIGBUS on the same variable. But I'll do the
change, no problem: the commit diff looks much simpler with the change
you proposed.

Thanks for the reviews!

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre

--
Paulo Zanoni