Hi Sergei,
On Tuesday, 16 January 2018 22:17:31 EET Sergei Shtylyov wrote:
> On 01/16/2018 06:46 PM, Laurent Pinchart wrote:
> >>> From: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
> >>>
> >>> According to the latest revision 2.00 of the R-Car gen2 manual, the LVDS
> >>> must be enabled and the bias crcuit enabled after the LVDS I/O pins are
>
> Oops, this needs fixing (note the typo!). Could you please change this
> passage to:
>
> and the bias circuit must be enabled after the LVDS I/O pins are
>
> ?
Sure I'll fix that.
> >>> enabled, not before. Fix the gen2 LVDS startup sequence accordingly.
> >>>
> >>> While at it, also fix the comment preceding the first LVDCR0 write that
> >>> still talks about hardcoding the LVDS mode 0.
> >>
> >> Please do this in a separate commit then...
> >
> > The reason I added it here is that I think we don't need patch 1/2 from
> > this series, and I found a bit overkill to split a comment fix to a
> > separate patch when we have a patch that touches the code around the
> > comment.
>
> OK, you're the maintainer of this driver, you know better. :-)
>
> >>> Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support")
> >>
> >> You forgot to specify the other commit this one fixes -- I mean the
> >> comment fix.
> >
> > Do we need to for a comment update ? It doesn't affect fix the behaviour
> > of the driver or device, and I'd thus prefer to avoid giving the wrong
> > impression that this patch fixes an bug introduced in a previous commit,
> > otherwise it might end up being backported unnecessarily.
>
> OK, no dire need to backport indeed...
>
> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
> >>> Reviewed-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> >>> [Set the mode and input at the same time as the BEN and LVEN bits]
> >>> Tested-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> >>> ---
> >>>
> >>> drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 14 +++++++-------
> >>> 1 file changed, 7 insertions(+), 7 deletions(-)
> >>>
> >>> Hi Sergei,
> >>>
> >>> For your convenience (and if you agree with bundling mode setup with the
> >>> first write as explained in my review of patch 1/2), here's the updated
> >>> version of patch 2/2 that I have taken in my development branch. If
> >>> you're fine with it I'll keep it, otherwise we can continue the review
> >>> discussion.
> >>
> >> As I said, I don't know how to interpret the note 3 in either manual.
>
> Moreover, it seems to me that the notes don't match the start-up procedure
> anymore...
How so ?
> > As explained in my latest reply to patch 1/2, my understanding is that the
> > parameters can be programmed at any time before step 6. The fact that the
> > current code works seems to confirm that interpretation. We could ask
> > Renesas for a confirmation if you want.
>
> Would be good to ask, indeed.
I'll ask.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
