[Intel-gfx] [PATCH 6/9] drm/i915: PCH_NOP suspend/resume

[Daniel Vetter]

On Tue, Mar 19, 2013 at 1:51 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Sun, Mar 17, 2013 at 10:28:55PM +0100, Daniel Vetter wrote:
>> On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote:
>> > More registers we can't write.
>> >
>> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> > ---
>> >  drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++-----------
>> >  1 file changed, 41 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>> > index c1e02b0..dd5766a 100644
>> > --- a/drivers/gpu/drm/i915/i915_suspend.c
>> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
>> > @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev)
>> >
>> >     mutex_lock(&dev->struct_mutex);
>> >
>> > -   i915_save_display(dev);
>> > +   if (!HAS_PCH_NOP(dev))
>> > +           i915_save_display(dev);
>>
>> This here looks a bit funny - imo it's better to move this check to the
>> only two places where we still touch registers in the kms case: lvds & pp
>> restore.
>
> I had something like this originally, except I also can't touch the
> backlight registers (even though they're not in a bad range).
>
> In an earlier patch you asked me to move the check up in the callchain,
> and I liked that idea. It seems to me here the same logic applies, we
> never care about any of the display registers. If you feel strongly
> about it though, I will change it. Please correct me if I misunderstood
> your request.

Yes, in general I think it's good to move the checks up in the
callchains and consolidate them that way. I'm voting for an exception
here in the register save/restore code since we're slowly moving away
from it for kms. Moving the PCH_NOP check into the few remaining
places allows us to easily kill them once those are guarded with if
(!kms) checks, too. I've just figured that that way we won't have a
stray PCH_NOP check left behind for code which (eventually) isn't run
at all in kms mode.

I don't mind if you leave this as-is, really just a tiny bikeshed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch