| Message ID | 1351531602-17161-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 29, 2012 at 5:26 PM, Jani Nikula <jani.nikula@intel.com> wrote: > Mask the value, not the register. Spotted while reading the code. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Good catch! FWIW, I ran: git grep -e "READ.*\(pipe\|plane\|cpu_transcoder\))[^)]*[|&]" drivers/gpu/drm/i915/ and this was the only catch.
On Mon, Oct 29, 2012 at 05:49:48PM +0000, Lespiau, Damien wrote: > On Mon, Oct 29, 2012 at 5:26 PM, Jani Nikula <jani.nikula@intel.com> wrote: > > Mask the value, not the register. Spotted while reading the code. > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Queued for -next, thanks for the patch. -Daniel
On Mon, Oct 29, 2012 at 07:26:42PM +0200, Jani Nikula wrote: > Mask the value, not the register. Spotted while reading the code. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > Only compile tested. Is there a bug that might match this? > --- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a5be346..6f8cf2a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2804,8 +2804,8 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc) > if (HAS_PCH_IBX(dev)) { > I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR); > I915_WRITE(FDI_RX_CHICKEN(pipe), > - I915_READ(FDI_RX_CHICKEN(pipe) & > - ~FDI_RX_PHASE_SYNC_POINTER_EN)); > + I915_READ(FDI_RX_CHICKEN(pipe)) & > + ~FDI_RX_PHASE_SYNC_POINTER_EN); On second look, we write the same register in the previous line, and the w/a seems to be to set FDI_RX_PHASE_SYNC_POINTER_OVR to enable the logic, then keep always set FDI_RX_PHASE_SYNC_POINTER_OVR and toggle ~FDI_RX_PHASE_SYNC_POINTER_EN before/after enabling the pc transcoder. So the right things seems to be to simply kill the 2nd write. -Daniel > } else if (HAS_PCH_CPT(dev)) { > cpt_phase_pointer_disable(dev, pipe); > } > -- > 1.7.9.5 >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a5be346..6f8cf2a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2804,8 +2804,8 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc) if (HAS_PCH_IBX(dev)) { I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR); I915_WRITE(FDI_RX_CHICKEN(pipe), - I915_READ(FDI_RX_CHICKEN(pipe) & - ~FDI_RX_PHASE_SYNC_POINTER_EN)); + I915_READ(FDI_RX_CHICKEN(pipe)) & + ~FDI_RX_PHASE_SYNC_POINTER_EN); } else if (HAS_PCH_CPT(dev)) { cpt_phase_pointer_disable(dev, pipe); }
Mask the value, not the register. Spotted while reading the code. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Only compile tested. Is there a bug that might match this? --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)