On 22/08/2022 16:20, Laurent Pinchart wrote:
Hi Tomi,
Thank you for the patch.
On Mon, Aug 22, 2022 at 04:05:10PM +0300, Tomi Valkeinen wrote:
From: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
Improve the DSI shutdown procedure by clearing various bits that were
set while enabling the DSI output. There has been no clear issues caused
by these, but it's safer to ensure that the features are disabled at the
start of the next DSI enable.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 7f2be490fcf8..6a10a35f1122 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -441,9 +441,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
static void rcar_mipi_dsi_shutdown(struct rcar_mipi_dsi *dsi)
{
+ /* Disable VCLKEN */
+ rcar_mipi_dsi_clr(dsi, VCLKEN, VCLKEN_CKEN);
+
+ /* Disable DOT clock */
+ rcar_mipi_dsi_clr(dsi, VCLKSET, VCLKSET_CKEN);
I think you can write 0 to those two registers, this will also be safer.
With this,
VCLKEN has only the single VCLKEN_CKEN bit and the rest of the bits are
reserved with default value of 0, however VCLKSET has other fields and
the default value of those fields is not 0.
Why do you think it's safer to set the whole register to 0? Isn't it
better to just do what we want to do, which makes the purpose clear and,
I think, is safer as we don't touch bits we don't know about?
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
I think there's a bug in rcar_mipi_dsi_startup() related to this by the
way, the function only uses rcar_mipi_dsi_set() to set bits, so if the
DSI format is modified between two starts, bad things will happen.
Oh, that's bad. rcar_mipi_dsi_set() is not a very good function as it's
easy to misuse it like that. I'll make a fix for that.
Tomi
