Re: [PATCH v5 1/5] drm: xlnx: Xilinx DRM KMS module

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hyun,

On Tuesday, 20 February 2018 19:11:42 EET hyun.kwon@xilinx.com wrote:
> On Monday, February 19, 2018 1:43 AM Daniel Vetter wrote:
> > On Tue, Feb 06, 2018 at 05:36:36PM -0800, Hyun Kwon wrote:
> >> Xilinx has various platforms for display, where users can create
> >> using multiple IPs in the programmable FPGA fabric, or where
> >> some hardened piepline is available on the chip. Furthermore,
> >> hardened pipeline can also interact with soft logics in FPGA.
> >> 
> >> The Xilinx DRM KMS module is to integrate multiple subdevices and
> >> to represent the entire pipeline as a single DRM device. The module
> >> includes helper (ex, framebuffer and gem helpers) and
> >> glue logic (ex, crtc interface) functions.
> >> 
> >> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com>
> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Looks all ready for merging. Did you apply for commit rights to drm-misc
> > already so you could push this right away?
> 
> Yes, I've created the request, and am waiting for the response there:
> https://bugs.freedesktop.org/show_bug.cgi?id=105017

I've just sent an in-depth review of patch 1/5 (sorry for being late). There 
are lots of small comments that could be addressed as follow-up patches in the 
worst case, but there's one comment regarding the ports DT property that 
worries me and that I'd like to see addressed (or, if I got it wrong, 
explained) before we merge this. Another related issue that I'd like to 
discuss is the need for the artificial xilinx-drm platform device. And of 
course if a v6 is needed, you can address all the other small comments :-)

> >> ---
> >> v5
> >> - Redefine xlnx_pipeline_init()
> >> v4
> >> - Fix a bug in of graph binding handling
> >> - Remove vblank callbacks from xlnx_crtc
> >> - Remove the dt binding. This module becomes more like a library.
> >> - Rephrase the commit message
> >> v3
> >> - Add Laurent as a maintainer
> >> - Fix multiple-reference on gem objects
> >> v2
> >> - Change the SPDX identifier format
> >> - Merge patches(crtc, gem, fb) into single one
> >> v2 of xlnx_drv
> >> - Rename kms to display in xlnx_drv
> >> - Replace some xlnx specific fb helper with common helpers in xlnx_drv
> >> - Don't set the commit tail callback in xlnx_drv
> >> - Support 'ports' graph binding in xlnx_drv
> >> v2 of xlnx_fb
> >> - Remove wrappers in xlnx_fb
> >> - Replace some functions with drm core helpers in xlnx_fb
> >> ---
> >> ---
> >> 
> >>  MAINTAINERS                      |   9 +
> >>  drivers/gpu/drm/Kconfig          |   2 +
> >>  drivers/gpu/drm/Makefile         |   1 +
> >>  drivers/gpu/drm/xlnx/Kconfig     |  12 +
> >>  drivers/gpu/drm/xlnx/Makefile    |   2 +
> >>  drivers/gpu/drm/xlnx/xlnx_crtc.c | 177 ++++++++++++++
> >>  drivers/gpu/drm/xlnx/xlnx_crtc.h |  70 ++++++
> >>  drivers/gpu/drm/xlnx/xlnx_drv.c  | 501 +++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/xlnx/xlnx_drv.h  |  33 +++
> >>  drivers/gpu/drm/xlnx/xlnx_fb.c   | 298 +++++++++++++++++++++++
> >>  drivers/gpu/drm/xlnx/xlnx_fb.h   |  33 +++
> >>  drivers/gpu/drm/xlnx/xlnx_gem.c  |  47 ++++
> >>  drivers/gpu/drm/xlnx/xlnx_gem.h  |  26 ++
> >>  13 files changed, 1211 insertions(+)
> >>  create mode 100644 drivers/gpu/drm/xlnx/Kconfig
> >>  create mode 100644 drivers/gpu/drm/xlnx/Makefile
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.c
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.h
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_drv.c
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_drv.h
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_fb.c
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_fb.h
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_gem.c
> >>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_gem.h

[snip]

-- 
Regards,

Laurent Pinchart

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