Re: [PATCH 2/2] drm/exynos: WARN_ON if ideal_clk is zero

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx>
---
 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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux