| Message ID | 1449660952-25222-1-git-send-email-m.deepak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote: > From: Gaurav K Singh <gaurav.k.singh@intel.com> > > Before sending TURN ON packet,check the DPI > FIFO empty status. > > v2: Change in commit message > Checking for FIFO empty only during TURN ON packet. > v3: Adding a new function for DPI FIFO empty check > > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > Signed-off-by: Deepak M <m.deepak@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 170ae6f..eff982b 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) > DRM_ERROR("DPI FIFOs are not empty\n"); > } > > +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) > +{ > + struct drm_encoder *encoder = &intel_dsi->base.base; > + struct drm_device *dev = encoder->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; Passing in intel_dsi just to dig out the dev_priv again seems a bit pointless. Could just pass dev_priv directly. But I guess it matches the already existing functions. BTW for future reference, we would rather see to_i915(<whatever>) instead of <whatever>->dev_private. Anyway, the patch seems to do what it says. I can't really judge whether it's the right thing to do though. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > + if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY) > + == DPI_FIFO_EMPTY, 50)) > + DRM_ERROR("Timeout waiting for DPI FIFO empty\n"); > +} > + > static void write_data(struct drm_i915_private *dev_priv, u32 reg, > const u8 *data, u32 len) > { > @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder) > I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4); > } else { > msleep(20); /* XXX */ > - for_each_dsi_port(port, intel_dsi->ports) > + for_each_dsi_port(port, intel_dsi->ports) { > + wait_for_dpi_fifo_empty(intel_dsi, port); > dpi_send_cmd(intel_dsi, TURN_ON, false, port); > + } > msleep(100); > > drm_panel_enable(intel_dsi->panel); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 09, 2015 at 04:34:54PM +0200, Ville Syrjälä wrote: > On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote: > > From: Gaurav K Singh <gaurav.k.singh@intel.com> > > > > Before sending TURN ON packet,check the DPI > > FIFO empty status. > > > > v2: Change in commit message > > Checking for FIFO empty only during TURN ON packet. > > v3: Adding a new function for DPI FIFO empty check > > > > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > > Signed-off-by: Deepak M <m.deepak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > > index 170ae6f..eff982b 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) > > DRM_ERROR("DPI FIFOs are not empty\n"); > > } > > > > +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) > > +{ > > + struct drm_encoder *encoder = &intel_dsi->base.base; > > + struct drm_device *dev = encoder->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > Passing in intel_dsi just to dig out the dev_priv again seems a bit > pointless. Could just pass dev_priv directly. But I guess it matches the > already existing functions. > > BTW for future reference, we would rather see to_i915(<whatever>) > instead of <whatever>->dev_private. Yeah I think fixing up the above would be good. We're pretty close to being able to embed our device structure. -Daniel > > Anyway, the patch seems to do what it says. I can't really judge > whether it's the right thing to do though. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > + > > + if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY) > > + == DPI_FIFO_EMPTY, 50)) > > + DRM_ERROR("Timeout waiting for DPI FIFO empty\n"); > > +} > > + > > static void write_data(struct drm_i915_private *dev_priv, u32 reg, > > const u8 *data, u32 len) > > { > > @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder) > > I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4); > > } else { > > msleep(20); /* XXX */ > > - for_each_dsi_port(port, intel_dsi->ports) > > + for_each_dsi_port(port, intel_dsi->ports) { > > + wait_for_dpi_fifo_empty(intel_dsi, port); > > dpi_send_cmd(intel_dsi, TURN_ON, false, port); > > + } > > msleep(100); > > > > drm_panel_enable(intel_dsi->panel); > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 09 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote: >> From: Gaurav K Singh <gaurav.k.singh@intel.com> >> >> Before sending TURN ON packet,check the DPI >> FIFO empty status. >> >> v2: Change in commit message >> Checking for FIFO empty only during TURN ON packet. >> v3: Adding a new function for DPI FIFO empty check >> >> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> >> Signed-off-by: Deepak M <m.deepak@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 170ae6f..eff982b 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) >> DRM_ERROR("DPI FIFOs are not empty\n"); >> } >> >> +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) >> +{ >> + struct drm_encoder *encoder = &intel_dsi->base.base; >> + struct drm_device *dev = encoder->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; > > Passing in intel_dsi just to dig out the dev_priv again seems a bit > pointless. Could just pass dev_priv directly. But I guess it matches the > already existing functions. > > BTW for future reference, we would rather see to_i915(<whatever>) > instead of <whatever>->dev_private. > > Anyway, the patch seems to do what it says. I can't really judge > whether it's the right thing to do though. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> OTOH, commit a799a9780eb5c874d9d7ca0bbee66401ca98c013 Author: Shobhit Kumar <shobhit.kumar@intel.com> Date: Thu Jul 3 16:35:40 2014 +0530 drm/i915/vlv: DPI FIFO empty check is not needed While sending DPI SHUTDOWN command, we cannot wait for FIFO empty as pipes are not disabled at that time. In case of MIPI we disable port first and send SHUTDOWN command while pipe is still running and FIFOs will not be empty, causing spurious error log Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> Tested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> + >> + if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY) >> + == DPI_FIFO_EMPTY, 50)) >> + DRM_ERROR("Timeout waiting for DPI FIFO empty\n"); >> +} >> + >> static void write_data(struct drm_i915_private *dev_priv, u32 reg, >> const u8 *data, u32 len) >> { >> @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder) >> I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4); >> } else { >> msleep(20); /* XXX */ >> - for_each_dsi_port(port, intel_dsi->ports) >> + for_each_dsi_port(port, intel_dsi->ports) { >> + wait_for_dpi_fifo_empty(intel_dsi, port); >> dpi_send_cmd(intel_dsi, TURN_ON, false, port); >> + } >> msleep(100); >> >> drm_panel_enable(intel_dsi->panel); >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 170ae6f..eff982b 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) DRM_ERROR("DPI FIFOs are not empty\n"); } +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port) +{ + struct drm_encoder *encoder = &intel_dsi->base.base; + struct drm_device *dev = encoder->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY) + == DPI_FIFO_EMPTY, 50)) + DRM_ERROR("Timeout waiting for DPI FIFO empty\n"); +} + static void write_data(struct drm_i915_private *dev_priv, u32 reg, const u8 *data, u32 len) { @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder) I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4); } else { msleep(20); /* XXX */ - for_each_dsi_port(port, intel_dsi->ports) + for_each_dsi_port(port, intel_dsi->ports) { + wait_for_dpi_fifo_empty(intel_dsi, port); dpi_send_cmd(intel_dsi, TURN_ON, false, port); + } msleep(100); drm_panel_enable(intel_dsi->panel);