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
