Hi Tomi,
Thank you for the patch.
On Mon, Sep 02, 2019 at 03:53:53PM +0300, Tomi Valkeinen wrote:
> Commit d49cd15550d9d4495f6187425318c245d58cb63f ("OMAPDSS: DISPC: lock
> access to DISPC_CONTROL & DISPC_CONFIG") added locking to
> mgr_fld_write(). This was needed in omapfb times due to lack of good
> locking, especially in the case of both V4L2 and fbdev layers using the
> DSS driver.
>
> This is not needed for omapdrm, so we can remove the locking.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
I've followed all code paths, and they end up either in the bridge
enable operations or the CRTC atomic flush (disregarding
suspend/resume). Those can't race each other, so this should be safe.
> ---
> drivers/gpu/drm/omapdrm/dss/dispc.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 785c5546067a..c6da33e7014f 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -184,9 +184,6 @@ struct dispc_device {
>
> struct regmap *syscon_pol;
> u32 syscon_pol_offset;
> -
> - /* DISPC_CONTROL & DISPC_CONFIG lock*/
> - spinlock_t control_lock;
> };
>
> enum omap_color_component {
> @@ -377,16 +374,7 @@ static void mgr_fld_write(struct dispc_device *dispc, enum omap_channel channel,
> enum mgr_reg_fields regfld, int val)
> {
> const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld];
While at it, should you turn this into a pointer to avoid an unnecessary
copy ?
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> - const bool need_lock = rfld.reg == DISPC_CONTROL || rfld.reg == DISPC_CONFIG;
> - unsigned long flags;
> -
> - if (need_lock) {
> - spin_lock_irqsave(&dispc->control_lock, flags);
> - REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low);
> - spin_unlock_irqrestore(&dispc->control_lock, flags);
> - } else {
> - REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low);
> - }
> + REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low);
> }
>
> static int dispc_get_num_ovls(struct dispc_device *dispc)
> @@ -4769,8 +4757,6 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
> platform_set_drvdata(pdev, dispc);
> dispc->dss = dss;
>
> - spin_lock_init(&dispc->control_lock);
> -
> /*
> * The OMAP3-based models can't be told apart using the compatible
> * string, use SoC device matching.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
