| Message ID | 1457455195-1938-13-git-send-email-sre@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sebastian, Thank you for the patch. On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: > This is a workaround for a hardware bug occuring > on OMAP3 with manually updated panels. Could you please explain what the bug is and how the workaround operates ? Do you have a reference to an errata document ? > Signed-off-By: Sebastian Reichel <sre@kernel.org> > --- > drivers/gpu/drm/omapdrm/omap_drv.h | 1 + > drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h > b/drivers/gpu/drm/omapdrm/omap_drv.h index 71e2c2284b86..3ab4919aff4b > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.h > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h > @@ -161,6 +161,7 @@ struct drm_plane *omap_plane_init(struct drm_device > *dev, int id, enum drm_plane_type type); > void omap_plane_install_properties(struct drm_plane *plane, > struct drm_mode_object *obj); > +void omap_plane_update_fifo(struct drm_plane *plane); > > struct drm_encoder *omap_encoder_init(struct drm_device *dev, > struct omap_dss_device *dssdev); > diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c > b/drivers/gpu/drm/omapdrm/omap_plane.c index d75b197eff46..0147e416140c > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_plane.c > +++ b/drivers/gpu/drm/omapdrm/omap_plane.c > @@ -75,6 +75,28 @@ static void omap_plane_cleanup_fb(struct drm_plane > *plane, omap_framebuffer_unpin(old_state->fb); > } > > +void omap_plane_update_fifo(struct drm_plane *plane) > +{ > + struct omap_plane *omap_plane = to_omap_plane(plane); > + struct drm_plane_state *state = plane->state; > + struct drm_device *dev = plane->dev; > + bool use_fifo_merge = false; > + u32 fifo_low, fifo_high; > + bool use_manual_update; > + > + if (!dispc_ovl_enabled(omap_plane->id)) > + return; Given that this function is called right after dispc_ovl_enable(omap_plane- >id, true), can this condition be true ? > + use_manual_update = omap_crtc_is_manual_updated(state->crtc); > + > + dispc_ovl_compute_fifo_thresholds(omap_plane->id, &fifo_low, &fifo_high, > + use_fifo_merge, use_manual_update); You can remove the use_fifo_merge variable and set the argument to false directly. > + > + dev_dbg(dev->dev, "update fifo: %d %d", fifo_low, fifo_high); The two variables are unsigned, you should use %u. > + dispc_ovl_set_fifo_threshold(omap_plane->id, fifo_low, fifo_high); On a side note, shouldn't the dispc_ovl_compute_fifo_thresholds() and dispc_ovl_set_fifo_threshold() functions be merged into a single one as they're always called together ? > +} > + > static void omap_plane_atomic_update(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > @@ -141,6 +163,7 @@ static void omap_plane_atomic_update(struct drm_plane > *plane, } > > dispc_ovl_enable(omap_plane->id, true); > + omap_plane_update_fifo(plane); > } > > static void omap_plane_atomic_disable(struct drm_plane *plane,
On 13/12/16 19:35, Laurent Pinchart wrote: > Hi Sebastian, > > Thank you for the patch. > > On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: >> This is a workaround for a hardware bug occuring >> on OMAP3 with manually updated panels. > > Could you please explain what the bug is and how the workaround operates ? Do > you have a reference to an errata document ? I don't think I ever found out exactly why the problem happens. But on OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if "manual_update" parameter is set on OMAP3. Tomi
Hi Tomi, On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote: > On 13/12/16 19:35, Laurent Pinchart wrote: > > On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: > >> This is a workaround for a hardware bug occuring > >> on OMAP3 with manually updated panels. > > > > Could you please explain what the bug is and how the workaround operates ? > > Do you have a reference to an errata document ? > > I don't think I ever found out exactly why the problem happens. But on > OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC > would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if > "manual_update" parameter is set on OMAP3. I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes sense to me. If Sebastian could address the small issues I pointed out, we could then merge this. Alternatively I can take care of addressing them.
On 14/12/16 11:10, Laurent Pinchart wrote: > Hi Tomi, > > On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote: >> On 13/12/16 19:35, Laurent Pinchart wrote: >>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: >>>> This is a workaround for a hardware bug occuring >>>> on OMAP3 with manually updated panels. >>> >>> Could you please explain what the bug is and how the workaround operates ? >>> Do you have a reference to an errata document ? >> >> I don't think I ever found out exactly why the problem happens. But on >> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC >> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if >> "manual_update" parameter is set on OMAP3. > > I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes > sense to me. If Sebastian could address the small issues I pointed out, we > could then merge this. Alternatively I can take care of addressing them. It's only needed with the rest of the DSI manual update series, so I'd rather keep it as part of that series. Tomi
Hi, On Wed, Dec 14, 2016 at 11:14:32AM +0200, Tomi Valkeinen wrote: > On 14/12/16 11:10, Laurent Pinchart wrote: > > Hi Tomi, > > > > On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote: > >> On 13/12/16 19:35, Laurent Pinchart wrote: > >>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: > >>>> This is a workaround for a hardware bug occuring > >>>> on OMAP3 with manually updated panels. > >>> > >>> Could you please explain what the bug is and how the workaround operates ? > >>> Do you have a reference to an errata document ? FWIW I don't know anything about this bug. I just hit it while getting omapdrm working on n950 and ported this over from omapfb. > >> I don't think I ever found out exactly why the problem happens. But on > >> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC > >> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if > >> "manual_update" parameter is set on OMAP3. > > > > I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes > > sense to me. If Sebastian could address the small issues I pointed out, we > > could then merge this. Alternatively I can take care of addressing them. > > It's only needed with the rest of the DSI manual update series, so I'd > rather keep it as part of that series. To be honest I haven't worked on this for some time. From some comments on the patchset I think the biggest issue is, that omapdrm does not use generic panel drivers and cannot easily use the mipi_dsi_driver_register. I simply did not have enough time to implement such a huge change. I guess a first step is Peter Ujfalusi's series: https://lkml.org/lkml/2016/9/1/267 -- Sebastian
Hi Sebastian, (CC'ing Peter Ujfalusi) On Wednesday 14 Dec 2016 15:03:08 Sebastian Reichel wrote: > On Wed, Dec 14, 2016 at 11:14:32AM +0200, Tomi Valkeinen wrote: > > On 14/12/16 11:10, Laurent Pinchart wrote: > >> On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote: > >>> On 13/12/16 19:35, Laurent Pinchart wrote: > >>>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote: > >>>>> This is a workaround for a hardware bug occuring > >>>>> on OMAP3 with manually updated panels. > >>>> > >>>> Could you please explain what the bug is and how the workaround > >>>> operates ? Do you have a reference to an errata document ? > > FWIW I don't know anything about this bug. I just hit it while > getting omapdrm working on n950 and ported this over from omapfb. I wonder if it's a bug or an expected behaviour. In any case, looking at the FIFO thresholds computation function it's quite clear that we need to recompute and set the values when the panel mode changes. Maybe you should explain this in the commit message. > >>> I don't think I ever found out exactly why the problem happens. But on > >>> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise > >>> DISPC would stop. dispc_ovl_compute_fifo_thresholds() does that tuning > >>> if "manual_update" parameter is set on OMAP3. > >> > >> I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch > >> makes sense to me. If Sebastian could address the small issues I pointed > >> out, we could then merge this. Alternatively I can take care of > >> addressing them. > > > > It's only needed with the rest of the DSI manual update series, so I'd > > rather keep it as part of that series. > > To be honest I haven't worked on this for some time. From some > comments on the patchset I think the biggest issue is, that omapdrm > does not use generic panel drivers and cannot easily use the > mipi_dsi_driver_register. I simply did not have enough time to > implement such a huge change. I'll probably give it a go at some point, but it will take time. > I guess a first step is Peter Ujfalusi's series: > https://lkml.org/lkml/2016/9/1/267 Peter, do you plan to respin that patch series ?
On 14/12/16 23:36, Laurent Pinchart wrote: >> I guess a first step is Peter Ujfalusi's series: >> https://lkml.org/lkml/2016/9/1/267 > > Peter, do you plan to respin that patch series ? That series has been merged already. Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index 71e2c2284b86..3ab4919aff4b 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -161,6 +161,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev, int id, enum drm_plane_type type); void omap_plane_install_properties(struct drm_plane *plane, struct drm_mode_object *obj); +void omap_plane_update_fifo(struct drm_plane *plane); struct drm_encoder *omap_encoder_init(struct drm_device *dev, struct omap_dss_device *dssdev); diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index d75b197eff46..0147e416140c 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -75,6 +75,28 @@ static void omap_plane_cleanup_fb(struct drm_plane *plane, omap_framebuffer_unpin(old_state->fb); } +void omap_plane_update_fifo(struct drm_plane *plane) +{ + struct omap_plane *omap_plane = to_omap_plane(plane); + struct drm_plane_state *state = plane->state; + struct drm_device *dev = plane->dev; + bool use_fifo_merge = false; + u32 fifo_low, fifo_high; + bool use_manual_update; + + if (!dispc_ovl_enabled(omap_plane->id)) + return; + + use_manual_update = omap_crtc_is_manual_updated(state->crtc); + + dispc_ovl_compute_fifo_thresholds(omap_plane->id, &fifo_low, &fifo_high, + use_fifo_merge, use_manual_update); + + dev_dbg(dev->dev, "update fifo: %d %d", fifo_low, fifo_high); + + dispc_ovl_set_fifo_threshold(omap_plane->id, fifo_low, fifo_high); +} + static void omap_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -141,6 +163,7 @@ static void omap_plane_atomic_update(struct drm_plane *plane, } dispc_ovl_enable(omap_plane->id, true); + omap_plane_update_fifo(plane); } static void omap_plane_atomic_disable(struct drm_plane *plane,
This is a workaround for a hardware bug occuring on OMAP3 with manually updated panels. Signed-off-By: Sebastian Reichel <sre@kernel.org> --- drivers/gpu/drm/omapdrm/omap_drv.h | 1 + drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+)