Hmm,
I wonder if that really 'fixes' anything, because now we get a WARN_ON
which is immediately followed by a div-by-zero. Furthermore we then
still use the result of that operation as input for a hw register (bad
idea?)
I thought of something like this. Change fimd_calc_clkdiv() to return a
signed value (this shouldn't be a problem, since the returned number
range is small anywary). Let fimd_calc_clkdiv() return -1 when it
encounters the 'ideal_clk == 0' case. Move computation of clkdiv in
fimd_commit() to the top of the function (right after the {h,v}total
checks). If 'clkdiv == -1' case is encountered, then do WARN_ON and
return immediately from fimd_commit().
Should I prepare a patch for this, or does someone see an issue with
this approach?
With best wishes,
Tobias
On 2015-05-20 16:33, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>
> ideal_clk is the divisor in an operation to find the clock divider so
> it can't never be zero or we will end up with a division by zero error.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 08f7197..294f9cf 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -309,6 +309,8 @@ static u32 fimd_calc_clkdiv(struct fimd_context
> *ctx,
> unsigned long ideal_clk = mode->htotal * mode->vtotal *
> mode->vrefresh;
> u32 clkdiv;
>
> + WARN_ON(ideal_clk == 0);
> +
> if (ctx->i80_if) {
> /*
> * The frame done interrupt should be occurred prior to the
