RE: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem

[Venkateshwar Rao Gannavarapu <VGANNAVA@xilinx.com>]

Hi Laurent,

Thanks for the review. 
Please see my comments about D-PHY and bridge driver implementation.

>-----Original Message-----
>From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>Sent: Sunday, June 7, 2020 7:55 AM
>To: Venkateshwar Rao Gannavarapu <VGANNAVA@xilinx.com>
>Cc: Hyun Kwon <hyunk@xilinx.com>; dri-devel@lists.freedesktop.org;
>airlied@linux.ie; daniel@ffwll.ch; linux-kernel@vger.kernel.org; Sandip Kothari
><sandipk@xilinx.com>
>Subject: Re: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem
>
>Hi GVRao,
>
>On Sun, May 31, 2020 at 05:41:50PM +0000, Venkateshwar Rao Gannavarapu
>wrote:
>> On Sunday, May 24, 2020 8:38 AM, Laurent Pinchart wrote:
>> > On Mon, May 04, 2020 at 11:43:48AM -0700, Hyun Kwon wrote:
>> >> On Mon, 2020-04-20 at 14:20:56 -0700, Venkateshwar Rao Gannavarapu
>wrote:
>> >>> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
>> >>> data from AXI-4 stream interface.
>> >>>
>> >>> It supports upto 4 lanes, optional register interface for the
>> >>> DPHY,
>> >>
>> >> I don't see the register interface for dphy support.
>> >
>> > I think the D-PHY should be supported through a PHY driver, as it
>> > seems to be shared between different subsystems.
>>
>> IP has the provision to read DPHY register for debug purpose only.
>> No programming of DPHY is required in subsystem.
>
>Do you know if this is the same D-PHY as used in the CSI2-RX subsystem ?
 
Same D-PHY core has been used in MIPI CSI2 RXSS, but with different configuration.
>
>> >>> multiple RGB color formats, command mode and video mode.
>> >>> This is a MIPI-DSI host driver and provides DSI bus for panels.
>> >>> This driver also helps to communicate with its panel using panel
>> >>> framework.
>> >>>
>> >>> Signed-off-by: Venkateshwar Rao Gannavarapu
>> >>> <venkateshwar.rao.gannavarapu@xilinx.com>
>> >>> ---
>> >>>  drivers/gpu/drm/xlnx/Kconfig    |  11 +
>> >>>  drivers/gpu/drm/xlnx/Makefile   |   2 +
>> >>>  drivers/gpu/drm/xlnx/xlnx_dsi.c | 755
>> >>> ++++++++++++++++++++++++++++++++++++++++
>> >
>> > Daniel Vetter has recently expressed his opiion that bridge drivers
>> > should go to drivers/gpu/drm/bridge/. It would then be
>> > drivers/gpu/drm/bridge/xlnx/. I don't have a strong opinion myself.

The DSI-TX subsystem IP block is not a bridge driver.
The DSI-TX subsystem IP block itself contains all the drm encoder/connector
functionality and it’s the last node in display pipe line. I didn't see any hard
requirement to implement it into bridge driver and I see many DSI drivers are
implemented as encoder drivers.
Xilinx PL DRM encoder drivers are implemented in modular approach so that
they can work with any CRTC driver which handles the DMA calls.
So, at this stage we want to upstream as encoder driver only.
>> >
>> >>>  3 files changed, 768 insertions(+)  create mode 100644
>> >>> drivers/gpu/drm/xlnx/xlnx_dsi.c
>
>[snip]
>
>--
>Regards,
>
>Laurent Pinchart

Regards,
GVRao