| Message ID | 1434664605-20419-1-git-send-email-gaurav.k.singh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 19, 2015 at 03:26:45AM +0530, Gaurav K Singh wrote: > During enable sequence for MIPI encoder in command mode, enable > MIPI display self-refresh mode bit in Pipe Ctrl reg. > > v2: Use crtc state flag instead of loop over encoders (Daniel) > > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index dd518d6..c53f66d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc) > return; > } > > + if (crtc->config->dsi_self_refresh) { > + val = val | PIPECONF_MIPI_DSR_ENABLE; > + I915_WRITE(reg, val); > + } Ah here it is. Please squash this patch with patch 7 so that you introduce the state tracking and the user for the new dsi_self_refresh bit in one patch. Makes reviewing a lot easier. Also please add a comment here that the additional write is required, and enforce ordering with a POSTING_READ. Thanks, Daniel > + > I915_WRITE(reg, val | PIPECONF_ENABLE); > POSTING_READ(reg); > } > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jun 22, 2015 at 02:05:51PM +0200, Daniel Vetter wrote: > On Fri, Jun 19, 2015 at 03:26:45AM +0530, Gaurav K Singh wrote: > > During enable sequence for MIPI encoder in command mode, enable > > MIPI display self-refresh mode bit in Pipe Ctrl reg. > > > > v2: Use crtc state flag instead of loop over encoders (Daniel) > > > > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index dd518d6..c53f66d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc) > > return; > > } > > > > + if (crtc->config->dsi_self_refresh) { > > + val = val | PIPECONF_MIPI_DSR_ENABLE; > > + I915_WRITE(reg, val); > > + } > > Ah here it is. Please squash this patch with patch 7 so that you introduce > the state tracking and the user for the new dsi_self_refresh bit in one > patch. Makes reviewing a lot easier. Also please add a comment here that > the additional write is required, and enforce ordering with a > POSTING_READ. And I just realized that I've written the same review in the discussion of the previous patch. Please make sure next time around you address all the outstanding review, since if you don't the reviewer has to paintstakingly check _everything_ every time around, which is a massive waste of everyone's time. Also can you pls change the subject to include DSI (and drop the MIPI part, that's just the standard's group and not the standard itself)? Applies to a few other patches in this series too. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dd518d6..c53f66d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc) return; } + if (crtc->config->dsi_self_refresh) { + val = val | PIPECONF_MIPI_DSR_ENABLE; + I915_WRITE(reg, val); + } + I915_WRITE(reg, val | PIPECONF_ENABLE); POSTING_READ(reg); }