Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

?

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...

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.

MBR, Sergei
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux