[PATCH v2] 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 13/10/17 12:35, Jyri Sarha wrote:
> We need the total 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 is inaccurate or may not
> even be initialized.
> 
> Reported-by: Kevin Hao <kexin.hao at windriver.com>
> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
> Cc: <stable at vger.kernel.org> # v4.11+
> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> ---
> Since first version:
> 
> Change frametime* variable names to hvtotal and use 64-bit division
> instead of dynamcally scaled 32-bit division as suggested by Tomi
> Valkeinen.
> 
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 704db24..a8b22aa 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -24,6 +24,7 @@
>  #include <linux/completion.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/of_graph.h>
> +#include <linux/math64.h>
>  
>  #include "tilcdc_drv.h"
>  #include "tilcdc_regs.h"
> @@ -48,6 +49,7 @@ struct tilcdc_crtc {
>  	unsigned int lcd_fck_rate;
>  
>  	ktime_t last_vblank;
> +	unsigned int hvtotal_us;
>  
>  	struct drm_framebuffer *curr_fb;
>  	struct drm_framebuffer *next_fb;
> @@ -292,6 +294,16 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
>  				LCDC_V2_CORE_CLK_EN);
>  }
>  
> +uint tilcdc_mode_hvtotal(const struct drm_display_mode *mode)
> +{
> +	uint ret;
> +
> +	ret = (uint) div_u64(1000llu * mode->htotal * mode->vtotal,
> +			     mode->clock);
> +
> +	return ret;
> +}

I don't think "uint" is recommended. Just use u32. And drop the ret
variable, just one-line return statement should be enough.

Otherwise:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ti.com>

 Tomi