On Fri, Jul 14, 2017 at 12:34:28PM +0300, Jani Nikula wrote: > On Wed, 12 Jul 2017, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > Quoting Dhinakaran Pandiyan (2017-07-12 09:47:25) > >> On Tuesday, July 11, 2017 3:19:53 PM PDT Jim Bride wrote: > >> > On SKL+ there is a bit in SRD_CTL that software is not supposed to > >> > modify, but we currently clobber that bit when we enable PSR. In > >> > order to preserve the value of that bit, go ahead and read SRD_CTL > >> > >> And which bit is that? Bit 29 (Context restore to PSR Active) in SRD_CTL. I'll add it to the commit message. It's worth noting that the bit is not technically reserved, but rather that SW is not allowed to change it. > > > > I think we would all be happier with keeping the explicit construction > > (and a smaller patch) if we used > > > > val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_CTL_RSVD_MASK; > > Agreed. Avoid read-modify-write as much as possible. I can do this if everyone thinks it's the thing to do, but it does open us up to a similar class of bug (B-Spec restricting mods to a bit / bit range after initial support for a platform was added) in the future. IMHO the code as written is safer. Jim > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx