diff mbox series

[2/5] drm: atmel-hlcdc: enable clock before configuring timing engine

Message ID 1575984287-26787-3-git-send-email-claudiu.beznea@microchip.com (mailing list archive)
State New, archived
Headers show
Series fixes for atmel-hlcdc | expand

Commit Message

Claudiu Beznea Dec. 10, 2019, 1:24 p.m. UTC
Changing pixel clock source without having this clock source enabled
will block the timing engine and the next operations after (in this case
setting ATMEL_HLCDC_CFG(5) settings in atmel_hlcdc_crtc_mode_set_nofb()
will fail). It is recomended (although in datasheet this is not present)
to actually enabled pixel clock source before doing any changes on timing
enginge (only SAM9X60 datasheet specifies that the peripheral clock and
pixel clock must be enabled before using LCD controller).

Fixes: 1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Sam Ravnborg Dec. 10, 2019, 8:18 p.m. UTC | #1
Hi Claudiu.

On Tue, Dec 10, 2019 at 03:24:44PM +0200, Claudiu Beznea wrote:
> Changing pixel clock source without having this clock source enabled
> will block the timing engine and the next operations after (in this case
> setting ATMEL_HLCDC_CFG(5) settings in atmel_hlcdc_crtc_mode_set_nofb()
> will fail). It is recomended (although in datasheet this is not present)
> to actually enabled pixel clock source before doing any changes on timing
> enginge (only SAM9X60 datasheet specifies that the peripheral clock and
> pixel clock must be enabled before using LCD controller).
> 
> Fixes: 1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

We already had a remotely similar fix.
See 262d67e73f9a920a20bd75278761400404a82de0
("drm: atmel-hlcdc: enable sys_clk during initalization.")

In this patch sys_clk is only enabled if we have a fixed_clk.
Maybe we should do this unconditionally in
atmel_hlcdc_dc_load()?

Then we do not need this enable(disable in the mode_set_nofb
implementation.

Have you considered this way to fix it?

	Sam

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 5040ed8d0871..721fa88bf71d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -73,7 +73,11 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  	unsigned long prate;
>  	unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>  	unsigned int cfg = 0;
> -	int div;
> +	int div, ret;
> +
> +	ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
> +	if (ret)
> +		return;
>  
>  	vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
>  	vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
> @@ -147,6 +151,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  			   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
>  			   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
>  			   cfg);
> +
> +	clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
>  }
>  
>  static enum drm_mode_status
> -- 
> 2.7.4
diff mbox series

Patch

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5040ed8d0871..721fa88bf71d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -73,7 +73,11 @@  static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 	unsigned long prate;
 	unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
 	unsigned int cfg = 0;
-	int div;
+	int div, ret;
+
+	ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+	if (ret)
+		return;
 
 	vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
 	vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
@@ -147,6 +151,8 @@  static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 			   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
 			   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
 			   cfg);
+
+	clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
 }
 
 static enum drm_mode_status