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

[Hyun Kwon <hyun.kwon@xilinx.com>]

Hi Laurent,

On Wed, 2018-02-21 at 15:22:31 -0800, Laurent Pinchart wrote:
> 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 :-)
> 

Thanks! I skimmed through your comments, and I prefer to clear and address all
in v6 before committing this. I've replied directly to those comments so we can
continue to discuss there.

Thanks,
-hyun

> > >> ---
> > >> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel