| Message ID | 20220822143401.135081-6-tomi.valkeinen@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: rcar-du: DSI fixes | expand |
Hi Tomi, Thank you for the patch. On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote: > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > > rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses > or-operation to add the new values to the current value in the register, > it should first make sure the fields are cleared. > > Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to > VCLKSET before the rest of the VCLKSET configuration. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > --- > drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > index 06250f2f3499..b60a6d4a5d46 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > return ret; > } > > - /* Enable DOT clock */ > - vclkset = VCLKSET_CKEN; > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > + /* Clear VCLKSET and enable DOT clock */ > + rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN); > + > + vclkset = 0; > > if (dsi_format == 24) > vclkset |= VCLKSET_BPP_24; You can replace one more set() with a write(): diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c index 06250f2f3499..2744ea23e6f6 100644 --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, /* Enable DOT clock */ vclkset = VCLKSET_CKEN; - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); if (dsi_format == 24) vclkset |= VCLKSET_BPP_24; @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div) | VCLKSET_LANE(dsi->lanes - 1); - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); /* After setting VCLKSET register, enable VCLKEN */ rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN); I'll apply patches 1/5 to 4/5 to my tree already.
On 22/08/2022 17:45, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote: >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> >> >> rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses >> or-operation to add the new values to the current value in the register, >> it should first make sure the fields are cleared. >> >> Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to >> VCLKSET before the rest of the VCLKSET configuration. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> >> --- >> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c >> index 06250f2f3499..b60a6d4a5d46 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c >> @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, >> return ret; >> } >> >> - /* Enable DOT clock */ >> - vclkset = VCLKSET_CKEN; >> - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); >> + /* Clear VCLKSET and enable DOT clock */ >> + rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN); >> + >> + vclkset = 0; >> >> if (dsi_format == 24) >> vclkset |= VCLKSET_BPP_24; > > You can replace one more set() with a write(): That's true. I'll send a new one. > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > index 06250f2f3499..2744ea23e6f6 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > > /* Enable DOT clock */ > vclkset = VCLKSET_CKEN; > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); > > if (dsi_format == 24) > vclkset |= VCLKSET_BPP_24; > @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div) > | VCLKSET_LANE(dsi->lanes - 1); > > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); > > /* After setting VCLKSET register, enable VCLKEN */ > rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN); > > I'll apply patches 1/5 to 4/5 to my tree already. I'm not sure if 4 works correctly without 2. Tomi
On Tue, Aug 23, 2022 at 01:52:38PM +0300, Tomi Valkeinen wrote: > On 22/08/2022 17:45, Laurent Pinchart wrote: > > Hi Tomi, > > > > Thank you for the patch. > > > > On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote: > >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > >> > >> rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses > >> or-operation to add the new values to the current value in the register, > >> it should first make sure the fields are cleared. > >> > >> Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to > >> VCLKSET before the rest of the VCLKSET configuration. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > >> --- > >> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > >> index 06250f2f3499..b60a6d4a5d46 100644 > >> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > >> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > >> @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > >> return ret; > >> } > >> > >> - /* Enable DOT clock */ > >> - vclkset = VCLKSET_CKEN; > >> - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > >> + /* Clear VCLKSET and enable DOT clock */ > >> + rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN); > >> + > >> + vclkset = 0; > >> > >> if (dsi_format == 24) > >> vclkset |= VCLKSET_BPP_24; > > > > You can replace one more set() with a write(): > > That's true. I'll send a new one. > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > > index 06250f2f3499..2744ea23e6f6 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > > @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > > > > /* Enable DOT clock */ > > vclkset = VCLKSET_CKEN; > > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > > + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); > > > > if (dsi_format == 24) > > vclkset |= VCLKSET_BPP_24; > > @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, > > vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div) > > | VCLKSET_LANE(dsi->lanes - 1); > > > > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > > + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); > > > > /* After setting VCLKSET register, enable VCLKEN */ > > rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN); > > > > I'll apply patches 1/5 to 4/5 to my tree already. > > I'm not sure if 4 works correctly without 2. I meant 1/5 to 4/5, not 1/5 and 4/5 :-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c index 06250f2f3499..b60a6d4a5d46 100644 --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, return ret; } - /* Enable DOT clock */ - vclkset = VCLKSET_CKEN; - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); + /* Clear VCLKSET and enable DOT clock */ + rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN); + + vclkset = 0; if (dsi_format == 24) vclkset |= VCLKSET_BPP_24;