Hi Sergei, Thank you for the patch. On Friday, 12 January 2018 22:12:04 EET Sergei Shtylyov wrote: > According to the latest revisions of the R-Car gen3 manual, the LVDS mode > must be set before the LVDS I/O pins are enabled, not after -- fix the > gen3 LVDS startup sequence accordingly... > > While at it, also fix the comment preceding the first LVDCR0 write in > the R-Car gen2 startup code that still talks about hardcoding the LVDS > mode 0... How about fixing that in patch 2/2 that touches the Gen2 initialization sequence ? I think I'd even go as far as squashing both patches, I don't think there's a need to split them. > Fixes: e947eccbeba4 ("drm: rcar-du: Add support for LVDS mode selection") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx> Is this really needed ? Does it fix a problem you've experienced, or is it theoretical only ? The mode shouldn't matter before the LVDS internal logic is turned on. Unless there's a real issue I'm not sure we should make the code more complex. > --- > drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > =================================================================== > --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > @@ -60,8 +60,8 @@ static void rcar_du_lvdsenc_start_gen2(s > rcar_lvds_write(lvds, LVDPLLCR, pllcr); > > /* > - * Select the input, hardcode mode 0, enable LVDS operation and turn > - * bias circuitry on. > + * Set the LVDS mode, select the input, enable LVDS operation, > + * and turn bias circuitry on. > */ > lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN; > if (rcrtc->index == 2) > @@ -106,6 +106,9 @@ static void rcar_du_lvdsenc_start_gen3(s > > rcar_lvds_write(lvds, LVDPLLCR, pllcr); > > + lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > + rcar_lvds_write(lvds, LVDCR0, lvdcr0); > + > /* Turn all the channels on. */ > rcar_lvds_write(lvds, LVDCR1, > LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) | > @@ -115,7 +118,8 @@ static void rcar_du_lvdsenc_start_gen3(s > * Turn the PLL on, set it to LVDS normal mode, wait for the startup > * delay and turn the output on. > */ > - lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON; > + > + lvdcr0 = | LVDCR0_PLLON; > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > lvdcr0 |= LVDCR0_PWD; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel