Xilinx DSI-TX subsytem consists of DSI controller core, AXI crossbar
and D-PHY as sub blocks. DSI TX subsystem driver supports multiple lanes
upto 4, RGB color formats, video mode and command modes.
DSI-TX driver is implemented as an encoder driver, as it going to be
the final node in display pipeline. Xilinx doesn't support any converter
logic to make this as bridge driver. Xilinx doesn't have such
use cases where end node can't be an encoder like DSI-TX. And Xilinx
encoder drivers represents a subsystem where individual blocks can't be
used with external components / encoders.
Venkateshwar Rao Gannavarapu (2):
dt-bindings: display: xlnx: dsi: This add a DT binding for Xilinx DSI
TX subsystem.
drm: xlnx: dsi: driver for Xilinx DSI TX subsystem
.../devicetree/bindings/display/xlnx/xlnx,dsi.yaml | 147 +++++
drivers/gpu/drm/xlnx/Kconfig | 11 +
drivers/gpu/drm/xlnx/Makefile | 2 +
drivers/gpu/drm/xlnx/xlnx_dsi.c | 701 +++++++++++++++++++++
4 files changed, 861 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml
create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
--
1.8.3.1
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
Hi Venkateshwar
On Tue, Aug 11, 2020 at 06:16:15AM +0530, Venkateshwar Rao Gannavarapu wrote:
> Xilinx DSI-TX subsytem consists of DSI controller core, AXI crossbar
> and D-PHY as sub blocks. DSI TX subsystem driver supports multiple lanes
> upto 4, RGB color formats, video mode and command modes.
>
> DSI-TX driver is implemented as an encoder driver, as it going to be
> the final node in display pipeline. Xilinx doesn't support any converter
> logic to make this as bridge driver. Xilinx doesn't have such
> use cases where end node can't be an encoder like DSI-TX. And Xilinx
> encoder drivers represents a subsystem where individual blocks can't be
> used with external components / encoders.
>
> Venkateshwar Rao Gannavarapu (2):
> dt-bindings: display: xlnx: dsi: This add a DT binding for Xilinx DSI
> TX subsystem.
> drm: xlnx: dsi: driver for Xilinx DSI TX subsystem
Can you please elaborate a little what was changed since v1.
Preferably in some dense list form where you also list who provided the
feedback, preferably in the individual patches.
This is a great help trying to trigger the memory of past reviewers.
Please also include feedback that was not addressed. For example Laurent
said that this is a bridge drive and thus belongs in bridge/ alas that
was not done for so please list a reason.
Sam
>
> .../devicetree/bindings/display/xlnx/xlnx,dsi.yaml | 147 +++++
> drivers/gpu/drm/xlnx/Kconfig | 11 +
> drivers/gpu/drm/xlnx/Makefile | 2 +
> drivers/gpu/drm/xlnx/xlnx_dsi.c | 701 +++++++++++++++++++++
> 4 files changed, 861 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml
> create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
>
> --
> 1.8.3.1
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Aug 11, 2020 at 2:46 AM Venkateshwar Rao Gannavarapu
<[email protected]> wrote:
>
> Xilinx DSI-TX subsytem consists of DSI controller core, AXI crossbar
> and D-PHY as sub blocks. DSI TX subsystem driver supports multiple lanes
> upto 4, RGB color formats, video mode and command modes.
>
> DSI-TX driver is implemented as an encoder driver, as it going to be
> the final node in display pipeline. Xilinx doesn't support any converter
> logic to make this as bridge driver. Xilinx doesn't have such
> use cases where end node can't be an encoder like DSI-TX. And Xilinx
> encoder drivers represents a subsystem where individual blocks can't be
> used with external components / encoders.
So there's indeed a bit of an argument about when it should just be
stitched together as components, and when it is better a bridge. But
bridges also come with plenty of convenience glue, and we're talking
about fpga so the point pretty much is to rewire this all eventually.
Maybe not by you folks, but people are r/e-ing these things so who
knows.
The other thing is you do have a bridge attached already, or should
have: The drm_panel. And you do get the entire drm-connector
instantiation rather wrong, at least the ->dpms hook is breaking
atomic real bad. The ->detect implementation is also quite the
adventure. So panel bridge is definitely required here.
Finally this also would avoid component.c usage in xlnx - not that
there's something wrong with that per se, but it always means driver
specific logic and interactions, so harder to maintain for people not
involved in the code base on a daily basis. Also I'm not sure how this
works, the component integration glue in the main driver seems to be
missing. And the upstream drm/xlnx doesn't seem to have it either.
So all together I do think drm_bridge sounds like the right thing to do here.
-Daniel
>
> Venkateshwar Rao Gannavarapu (2):
> dt-bindings: display: xlnx: dsi: This add a DT binding for Xilinx DSI
> TX subsystem.
> drm: xlnx: dsi: driver for Xilinx DSI TX subsystem
>
> .../devicetree/bindings/display/xlnx/xlnx,dsi.yaml | 147 +++++
> drivers/gpu/drm/xlnx/Kconfig | 11 +
> drivers/gpu/drm/xlnx/Makefile | 2 +
> drivers/gpu/drm/xlnx/xlnx_dsi.c | 701 +++++++++++++++++++++
> 4 files changed, 861 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml
> create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
>
> --
> 1.8.3.1
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Hi GVRao,
Thank you for the patches.
On Tue, Aug 11, 2020 at 06:16:15AM +0530, Venkateshwar Rao Gannavarapu wrote:
> Xilinx DSI-TX subsytem consists of DSI controller core, AXI crossbar
> and D-PHY as sub blocks. DSI TX subsystem driver supports multiple lanes
> upto 4, RGB color formats, video mode and command modes.
>
> DSI-TX driver is implemented as an encoder driver, as it going to be
> the final node in display pipeline. Xilinx doesn't support any converter
> logic to make this as bridge driver. Xilinx doesn't have such
> use cases where end node can't be an encoder like DSI-TX. And Xilinx
> encoder drivers represents a subsystem where individual blocks can't be
> used with external components / encoders.
>
> Venkateshwar Rao Gannavarapu (2):
> dt-bindings: display: xlnx: dsi: This add a DT binding for Xilinx DSI
> TX subsystem.
> drm: xlnx: dsi: driver for Xilinx DSI TX subsystem
>
> .../devicetree/bindings/display/xlnx/xlnx,dsi.yaml | 147 +++++
> drivers/gpu/drm/xlnx/Kconfig | 11 +
> drivers/gpu/drm/xlnx/Makefile | 2 +
> drivers/gpu/drm/xlnx/xlnx_dsi.c | 701 +++++++++++++++++++++
> 4 files changed, 861 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml
> create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
>
> --
> 1.8.3.1
>
> This email and any attachments are intended for the sole use of the
> named recipient(s) and contain(s) confidential information that may be
> proprietary, privileged or copyrighted under applicable law. If you
> are not the intended recipient, do not read, copy, or forward this
> email message or any attachments. Delete this email message and any
> attachments immediately.
Unrelated to the technical contents of this series, this footer makes no
sense for upstream submissions. It's actually a legal issue, and I know
several maintainers who would delete the e-mails without even looking at
them due to this (Greg KH has stated this publicly before for instance).
I assume this is added by the mail server, but it would be good if
someone could get in touch with the IT department to see how this can be
dropped for patches sent to mailing lists. There's no specific urgency,
it can be a background task.
--
Regards,
Laurent Pinchart