Re: [PATCH 2/2] OMAPDSS: APPLY: make ovl_enable/disable synchronous

[Florian Tobias Schandinat <FlorianSchandinat@gmx.de>]

On 02/29/2012 08:48 AM, Tomi Valkeinen wrote:
> ovl->enable/disable are meant to be synchronous so that they can handle
> the configuration of fifo sizes. The current kernel doesn't configure
> fifo sizes yet, and so the code doesn't need to block to function (from
> omapdss driver's perspective).
> 
> However, for the users of omapdss a non-blocking ovl->disable is
> confusing, because they don't know when the memory area is not used
> any more.
> 
> Furthermore, when the fifo size configuration is added in the next merge
> window, the change from non-blocking to blocking could cause side
> effects to the users of omapdss. So by making the functions block
> already will keep them behaving in the same manner.

Is there any difference to doing it now?
I agree that this should be fixed but if we can't avoid breaking users I'd
prefer to break them in a merge window not in late rc stage. Or did we introduce
these functions just in the last merge window?


Best regards,

Florian Tobias Schandinat

> 
> And, while not the main purpose of this patch, this will also remove the
> compile warning:
> 
> drivers/video/omap2/dss/apply.c:350: warning:
> 'wait_pending_extra_info_updates' defined but not used
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/video/omap2/dss/apply.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 052dc87..87b3e25 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -1276,6 +1276,9 @@ int dss_ovl_enable(struct omap_overlay *ovl)
>  
>  	spin_unlock_irqrestore(&data_lock, flags);
>  
> +	/* wait for overlay to be enabled */
> +	wait_pending_extra_info_updates();
> +
>  	mutex_unlock(&apply_lock);
>  
>  	return 0;
> @@ -1313,6 +1316,9 @@ int dss_ovl_disable(struct omap_overlay *ovl)
>  
>  	spin_unlock_irqrestore(&data_lock, flags);
>  
> +	/* wait for the overlay to be disabled */
> +	wait_pending_extra_info_updates();
> +
>  	mutex_unlock(&apply_lock);
>  
>  	return 0;