[PATCH] drm/tilcdc: Precalculate total frametime in tilcdc_crtc_set_mode()

[PATCH] drm/tilcdc: Precalculate total frametime in tilcdc_crtc_set_mode()

[Jyri Sarha]

We need the frame refresh time to check if we are too close to
vertical sync when updating the two framebuffer DMA registers and risk
a collision. This new method is more accurate that the previous that
based on mode's vrefresh value, which itself may also be inaccurate
or not even initialized.

Reported-by: Kevin Hao <kexin.hao@windriver.com>
Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
This patch is inteded to replace this:
https://lists.freedesktop.org/archives/dri-devel/2017-October/154556.html

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 704db24..b2f2170 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -48,6 +48,7 @@ struct tilcdc_crtc {
 	unsigned int lcd_fck_rate;
 
 	ktime_t last_vblank;
+	unsigned int frametime_us;
 
 	struct drm_framebuffer *curr_fb;
 	struct drm_framebuffer *next_fb;
@@ -292,6 +293,16 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 				LCDC_V2_CORE_CLK_EN);
 }
 
+u32 tilcdc_mode_frametime(const struct drm_display_mode *mode)
+{
+	u32 totalframes = mode->htotal * mode->vtotal;
+
+	if (totalframes < U32_MAX / 1000u)
+		return (1000u * totalframes) / mode->clock;
+	else
+		return 10u * ((100u * totalframes) / mode->clock);
+}
+
 static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
@@ -459,6 +470,9 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 	drm_framebuffer_get(fb);
 
 	crtc->hwmode = crtc->state->adjusted_mode;
+
+	tilcdc_crtc->frametime_us =
+		tilcdc_mode_frametime(&crtc->hwmode);
 }
 
 static void tilcdc_crtc_enable(struct drm_crtc *crtc)
@@ -642,7 +656,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags);
 
 		next_vblank = ktime_add_us(tilcdc_crtc->last_vblank,
-					   1000000 / crtc->hwmode.vrefresh);
+					   tilcdc_crtc->frametime_us);
 		tdiff = ktime_to_us(ktime_sub(next_vblank, ktime_get()));
 
 		if (tdiff < TILCDC_VBLANK_SAFETY_THRESHOLD_US)
-- 
1.9.1


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH] drm/tilcdc: Precalculate total frametime in tilcdc_crtc_set_mode()

[Tomi Valkeinen]


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 12/10/17 17:16, Jyri Sarha wrote:
> We need the frame refresh time to check if we are too close to
> vertical sync when updating the two framebuffer DMA registers and risk
> a collision. This new method is more accurate that the previous that
> based on mode's vrefresh value, which itself may also be inaccurate
> or not even initialized.
> 
> Reported-by: Kevin Hao <kexin.hao@windriver.com>
> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
> Cc: <stable@vger.kernel.org> # v4.11+
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> This patch is inteded to replace this:
> https://lists.freedesktop.org/archives/dri-devel/2017-October/154556.html
> 
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 704db24..b2f2170 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -48,6 +48,7 @@ struct tilcdc_crtc {
>  	unsigned int lcd_fck_rate;
>  
>  	ktime_t last_vblank;
> +	unsigned int frametime_us;
>  
>  	struct drm_framebuffer *curr_fb;
>  	struct drm_framebuffer *next_fb;
> @@ -292,6 +293,16 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
>  				LCDC_V2_CORE_CLK_EN);
>  }
>  
> +u32 tilcdc_mode_frametime(const struct drm_display_mode *mode)
> +{
> +	u32 totalframes = mode->htotal * mode->vtotal;

"totalframes"? Shouldn't it be total pixels (including blanking pixels)?
Or maybe "hvtotal", mimicking the htotal and vtotal.

> +
> +	if (totalframes < U32_MAX / 1000u)
> +		return (1000u * totalframes) / mode->clock;
> +	else
> +		return 10u * ((100u * totalframes) / mode->clock);

Just pick one, I don't think there's benefit in the above if. Perhaps go
via u64 div.

 Tomi