On 25/06/13 17:22, Darren Etheridge wrote:
> From: Afzal Mohammed <afzal@xxxxxx>
>
> fb_set_par helps in runtime configuration of lcd controller like
> changing resolution, pixel clock etc. (eg. using fbset utility)
>
> Reconfigure lcd controller based on information passed by framework.
> Enable raster back if it was already enabled.
>
> As fb_set_par would get invoked indirectly from probe via fb_set_var,
> remove existing lcdc initialization in probe and do lcdc reset in
> probe so that reset happens only at the begining.
>
> Signed-off-by: Afzal Mohammed <afzal@xxxxxx>
> Signed-off-by: Darren Etheridge <detheridge@xxxxxx>
> ---
> drivers/video/da8xx-fb.c | 60 +++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 49 insertions(+), 11 deletions(-)
>
<snip>
> @@ -1201,9 +1201,52 @@ static int da8xx_pan_display(struct fb_var_screeninfo *var,
> return ret;
> }
>
> +static int da8xxfb_set_par(struct fb_info *info)
> +{
> + struct da8xx_fb_par *par = info->par;
> + int ret;
> + bool raster = da8xx_fb_is_raster_enabled();
> +
> + if (raster)
> + lcd_disable_raster(true);
> + else
> + lcd_disable_raster(false);
Not actually part of this patch, but: that kind of functions are quite
unclear. What does disable(true) mean? You have to read the function to
understand the call.
I think it'd be much better to take the wait-for-frame-done code out
from the lcd_disable_raster function, and call the wait function
explicitly here in the if (raster) block.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
