* [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-16 20:22 ` Dario Binacchi
0 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-16 20:22 UTC (permalink / raw)
To: linux-kernel
Cc: Dario Binacchi, Daniel Vetter, David Airlie, Jyri Sarha,
Tomi Valkeinen, dri-devel
The fdd property of the tilcdc_panel_info structure must set the reqdly
bit field (bit 12 to 19) of the raster control register. The previous
statement set the least significant bit instead.
Signed-off-by: Dario Binacchi <dariobin@libero.it>
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..238068e28729 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
return;
}
}
- reg |= info->fdd < 12;
+ reg |= info->fdd << 12;
tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
if (info->invert_pxl_clk)
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-16 20:22 ` Dario Binacchi
0 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-16 20:22 UTC (permalink / raw)
To: linux-kernel
Cc: David Airlie, Tomi Valkeinen, dri-devel, Jyri Sarha, Dario Binacchi
The fdd property of the tilcdc_panel_info structure must set the reqdly
bit field (bit 12 to 19) of the raster control register. The previous
statement set the least significant bit instead.
Signed-off-by: Dario Binacchi <dariobin@libero.it>
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..238068e28729 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
return;
}
}
- reg |= info->fdd < 12;
+ reg |= info->fdd << 12;
tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
if (info->invert_pxl_clk)
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
2021-02-16 20:22 ` Dario Binacchi
@ 2021-02-17 6:41 ` Tomi Valkeinen
-1 siblings, 0 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2021-02-17 6:41 UTC (permalink / raw)
To: Dario Binacchi, linux-kernel, Jyri Sarha
Cc: Daniel Vetter, David Airlie, dri-devel
On 16/02/2021 22:22, Dario Binacchi wrote:
> The fdd property of the tilcdc_panel_info structure must set the reqdly
> bit field (bit 12 to 19) of the raster control register. The previous
> statement set the least significant bit instead.
>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
>
> ---
>
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 30213708fc99..238068e28729 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> return;
> }
> }
> - reg |= info->fdd < 12;
> + reg |= info->fdd << 12;
> tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
>
> if (info->invert_pxl_clk)
>
This is interesting, looks like this has always been broken, and in many
cases sets bits 0, which is the enable bit. So we enable LCDC before
even setting the fb address. How does this not blow up LCDC totally?
The fix looks correct to me, but it will change the register value for
boards that have apparently been working for years.
Dario, did you test this on actual HW, or did you just spot the error?
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-17 6:41 ` Tomi Valkeinen
0 siblings, 0 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2021-02-17 6:41 UTC (permalink / raw)
To: Dario Binacchi, linux-kernel, Jyri Sarha; +Cc: David Airlie, dri-devel
On 16/02/2021 22:22, Dario Binacchi wrote:
> The fdd property of the tilcdc_panel_info structure must set the reqdly
> bit field (bit 12 to 19) of the raster control register. The previous
> statement set the least significant bit instead.
>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
>
> ---
>
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 30213708fc99..238068e28729 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> return;
> }
> }
> - reg |= info->fdd < 12;
> + reg |= info->fdd << 12;
> tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
>
> if (info->invert_pxl_clk)
>
This is interesting, looks like this has always been broken, and in many
cases sets bits 0, which is the enable bit. So we enable LCDC before
even setting the fb address. How does this not blow up LCDC totally?
The fix looks correct to me, but it will change the register value for
boards that have apparently been working for years.
Dario, did you test this on actual HW, or did you just spot the error?
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
2021-02-17 6:41 ` Tomi Valkeinen
@ 2021-02-17 17:45 ` Dario Binacchi
-1 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-17 17:45 UTC (permalink / raw)
To: Tomi Valkeinen, linux-kernel, Jyri Sarha; +Cc: David Airlie, dri-devel
Hi Tomi,
> Il 17/02/2021 07:41 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> ha scritto:
>
>
> On 16/02/2021 22:22, Dario Binacchi wrote:
> > The fdd property of the tilcdc_panel_info structure must set the reqdly
> > bit field (bit 12 to 19) of the raster control register. The previous
> > statement set the least significant bit instead.
> >
> > Signed-off-by: Dario Binacchi <dariobin@libero.it>
> >
> > ---
> >
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > index 30213708fc99..238068e28729 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> > return;
> > }
> > }
> > - reg |= info->fdd < 12;
> > + reg |= info->fdd << 12;
> > tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
> >
> > if (info->invert_pxl_clk)
> >
>
> This is interesting, looks like this has always been broken, and in many
> cases sets bits 0, which is the enable bit. So we enable LCDC before
> even setting the fb address. How does this not blow up LCDC totally?
>
> The fix looks correct to me, but it will change the register value for
> boards that have apparently been working for years.
>
> Dario, did you test this on actual HW, or did you just spot the error?
I tested it on Beaglebone Black + LCD cape (4.3inch).
I also checked the register value with devmem.
Regards,
Dario
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
> Tomi
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-17 17:45 ` Dario Binacchi
0 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-17 17:45 UTC (permalink / raw)
To: Tomi Valkeinen, linux-kernel, Jyri Sarha; +Cc: David Airlie, dri-devel
Hi Tomi,
> Il 17/02/2021 07:41 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> ha scritto:
>
>
> On 16/02/2021 22:22, Dario Binacchi wrote:
> > The fdd property of the tilcdc_panel_info structure must set the reqdly
> > bit field (bit 12 to 19) of the raster control register. The previous
> > statement set the least significant bit instead.
> >
> > Signed-off-by: Dario Binacchi <dariobin@libero.it>
> >
> > ---
> >
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > index 30213708fc99..238068e28729 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> > return;
> > }
> > }
> > - reg |= info->fdd < 12;
> > + reg |= info->fdd << 12;
> > tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
> >
> > if (info->invert_pxl_clk)
> >
>
> This is interesting, looks like this has always been broken, and in many
> cases sets bits 0, which is the enable bit. So we enable LCDC before
> even setting the fb address. How does this not blow up LCDC totally?
>
> The fix looks correct to me, but it will change the register value for
> boards that have apparently been working for years.
>
> Dario, did you test this on actual HW, or did you just spot the error?
I tested it on Beaglebone Black + LCD cape (4.3inch).
I also checked the register value with devmem.
Regards,
Dario
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
> Tomi
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
2021-02-16 20:22 ` Dario Binacchi
@ 2021-02-18 16:00 ` Jyri Sarha
-1 siblings, 0 replies; 10+ messages in thread
From: Jyri Sarha @ 2021-02-18 16:00 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, Daniel Vetter, David Airlie, Tomi Valkeinen, dri-devel
On 2021-02-16 22:22, Dario Binacchi wrote:
> The fdd property of the tilcdc_panel_info structure must set the reqdly
> bit field (bit 12 to 19) of the raster control register. The previous
> statement set the least significant bit instead.
>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
Reviewed-by: Jyri Sarha <jyri.sarha@iki.fi>
Tested-by: Jyri Sarha <jyri.sarha@iki.fi>
Thanks for a good catch. I'll merge to this drm-misc-next soon.
Best regards,
Jyri
>
> ---
>
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 30213708fc99..238068e28729 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc
> *crtc)
> return;
> }
> }
> - reg |= info->fdd < 12;
> + reg |= info->fdd << 12;
> tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
>
> if (info->invert_pxl_clk)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-18 16:00 ` Jyri Sarha
0 siblings, 0 replies; 10+ messages in thread
From: Jyri Sarha @ 2021-02-18 16:00 UTC (permalink / raw)
To: Dario Binacchi; +Cc: David Airlie, Tomi Valkeinen, dri-devel, linux-kernel
On 2021-02-16 22:22, Dario Binacchi wrote:
> The fdd property of the tilcdc_panel_info structure must set the reqdly
> bit field (bit 12 to 19) of the raster control register. The previous
> statement set the least significant bit instead.
>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
Reviewed-by: Jyri Sarha <jyri.sarha@iki.fi>
Tested-by: Jyri Sarha <jyri.sarha@iki.fi>
Thanks for a good catch. I'll merge to this drm-misc-next soon.
Best regards,
Jyri
>
> ---
>
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 30213708fc99..238068e28729 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc
> *crtc)
> return;
> }
> }
> - reg |= info->fdd < 12;
> + reg |= info->fdd << 12;
> tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
>
> if (info->invert_pxl_clk)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-16 20:17 ` Dario Binacchi
0 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-16 20:17 UTC (permalink / raw)
To: linux-kernel
Cc: Dario Binacchi, Daniel Vetter, David Airlie, Jyri Sarha,
Tomi Valkeinen, dri-devel
The fdd property of the tilcdc_panel_info structure must set the reqdly
bit field (bit 12 to 19) of the raster control register. The previous
statement set the least significant bit instead.
Signed-off-by: Dario Binacchi <dariobin@libero.it>
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..238068e28729 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
return;
}
}
- reg |= info->fdd < 12;
+ reg |= info->fdd << 12;
tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
if (info->invert_pxl_clk)
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RESEND PATCH] drm/tilcdc: fix raster control register setting
@ 2021-02-16 20:17 ` Dario Binacchi
0 siblings, 0 replies; 10+ messages in thread
From: Dario Binacchi @ 2021-02-16 20:17 UTC (permalink / raw)
To: linux-kernel
Cc: David Airlie, dri-devel, Tomi Valkeinen, Jyri Sarha, Dario Binacchi
The fdd property of the tilcdc_panel_info structure must set the reqdly
bit field (bit 12 to 19) of the raster control register. The previous
statement set the least significant bit instead.
Signed-off-by: Dario Binacchi <dariobin@libero.it>
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..238068e28729 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -393,7 +393,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
return;
}
}
- reg |= info->fdd < 12;
+ reg |= info->fdd << 12;
tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
if (info->invert_pxl_clk)
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-18 18:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 20:22 [RESEND PATCH] drm/tilcdc: fix raster control register setting Dario Binacchi
2021-02-16 20:22 ` Dario Binacchi
2021-02-17 6:41 ` Tomi Valkeinen
2021-02-17 6:41 ` Tomi Valkeinen
2021-02-17 17:45 ` Dario Binacchi
2021-02-17 17:45 ` Dario Binacchi
2021-02-18 16:00 ` Jyri Sarha
2021-02-18 16:00 ` Jyri Sarha
-- strict thread matches above, loose matches on Subject: below --
2021-02-16 20:17 Dario Binacchi
2021-02-16 20:17 ` Dario Binacchi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.