On Tue, Mar 19, 2013 at 08:51:01AM +0100, Daniel Vetter wrote: > 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 I think having the check never hurts, and since I tend to introduce bugs whenever I change things, I'll punt on this if you don't mind. > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center
