Messages in this thread | | | From | Laurent Pinchart <> | Subject | Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update | Date | Tue, 13 Dec 2016 19:35:48 +0200 |
| |
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,
-- Regards,
Laurent Pinchart
| |