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 at 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
