On Mon, Nov 25, 2013 at 06:55:52PM -0200, Paulo Zanoni wrote: > 2013/11/21 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > > On Thu, Nov 21, 2013 at 01:47:21PM -0200, Paulo Zanoni wrote: > >> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > >> > >> 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@xxxxxxxxx> > >> --- > >> 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. The way I think about it is that this function is exceptional. We typically use a common error code that we percolate all the way back through the stack, and so ret = callee(); if (ret) return ret; is a clean idiom. The difference here is just that we need to translate from one enum to another, rather like a ERR_PTR() cast. Instead of a tidy inline function, we have the conversion as an explicit switch, but if you think of it just as return ERR_FAULT(ret), you can see how it fits into our typical idiom. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
