Re: [PATCH 41/65] OMAPDSS: APPLY: add missing uses of spinlock

Tomi Valkeinen
2011-11-23T10:12:33Z

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

On Wed, 2011-11-23 at 15:26 +0530, Archit Taneja wrote:
> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:

> > int dss_mgr_set_device(struct omap_overlay_manager *mgr,
> > @@ -745,16 +762,28 @@ err:
> > int dss_ovl_set_info(struct omap_overlay *ovl,
> > struct omap_overlay_info *info)
> > {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&data_lock, flags);
> > +
> > ovl->info = *info;
> > ovl->info_dirty = true;
> >
> > + spin_unlock_irqrestore(&data_lock, flags);
> > +
> > return 0;
> > }
> >
> > void dss_ovl_get_info(struct omap_overlay *ovl,
> > struct omap_overlay_info *info)
> > {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&data_lock, flags);
> > +
> > *info = ovl->info;
> > +
> > + spin_unlock_irqrestore(&data_lock, flags);
> > }
>
> The get/set info functions for overlays and managers only modify the
> omap_overlay_info or manager_info structs, these aren't really a part of
> 'dss_data', they only become a part of dss_data only when we call
> mgr->apply().
>
> So, are we protecting these functions so that 2 users of the same
> overlay don't see incorrect info values?

True, at this point the data_lock is a bit vague, and is protecting also
the info fields in omap_overlay and omap_overlay_manager.

A lock is needed, though, as otherwise the info struct may be only
partial. E.g. somebody calls set_info, which is half way copying the
values, and somebody else calls apply or get_info.

In the next patches the infos will be moved into the dss_data, and then
using dss_lock spin lock makes more sense.

Tomi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]