On 22/08/2022 17:05, Laurent Pinchart wrote:
On Mon, Aug 22, 2022 at 04:49:02PM +0300, Tomi Valkeinen wrote:
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.
But the two fields whose default value isn't 0 are set in the startup()
function (albeit incorrectly as discussed below), so it should be fine.
That is true. But I'd rather write 0 to VCLKEN in the startup, before
writing the configuration.
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?
Because it will ensure that we don't get surprises when we later restart
the device, such as mentioned below :-)
Well, but that's a bug in the startup code. I don't think the shutdown
code should do things to make startup work better if the startup does
something wrong. Nevertheless, while I slightly disagree, I'm fine with
writing zero there in shutdown.
Tomi
