Re: [Intel-gfx] [PATCH v2 00/59] Add support for KeemBay DRM driver

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 15, 2020 at 05:05:49PM +0200, Daniel Vetter wrote:
> Hi Anitha
> 
> On Tue, Jul 14, 2020 at 01:56:46PM -0700, Anitha Chrisanthus wrote:
> > This is a new DRM driver for Intel's KeemBay SOC.
> > The SoC couples an ARM Cortex A53 CPU with an Intel
> > Movidius VPU.
> > 
> > This driver is tested with the KMB EVM board which is the refernce baord
> > for Keem Bay SOC. The SOC's display pipeline is as follows
> > 
> > +--------------+    +---------+    +-----------------------+
> > |LCD controller| -> |Mipi DSI | -> |Mipi to HDMI Converter |
> > +--------------+    +---------+    +-----------------------+
> > 
> > LCD controller and Mipi DSI transmitter are part of the SOC and
> > mipi to HDMI converter is ADV7535 for KMB EVM board.
> > 
> > The DRM driver is a basic KMS atomic modesetting display driver and
> > has no 2D or 3D graphics.It calls into the ADV bridge driver at
> > the connector level.
> > 
> > Only 1080p resolution and single plane is supported at this time.
> > The usecase is for debugging video and camera outputs.
> > 
> > Device tree patches are under review here
> > https://lore.kernel.org/linux-arm-kernel/20200708175020.194436-1-daniele.alessandrelli@linux.intel.com/T/
> 
> Cool, new driver, thanks a lot for submitting.
> 
> > Changes since v1:
> > - Removed redundant license text, updated license
> > - Rearranged include blocks
> > - renamed global vars and removed extern in c
> > - Used upclassing for dev_private
> > - Used drm_dev_init in drm device create (will be updated to use
> >   devm_drm_dev_alloc() in a separate patch later as kmb driver is currently
> >   developed on 5.4 kernel)
> 
> drm moves fairly quickly, please develop the upstream submission on top of
> linux-next or similar. We constantly add new helpers to simplify drivers,
> and we expect new driver submissions to be up to date with all that.
> 
> Another thing: From your description it sounds like it's a very simple
> driver, just a single plane/crtc, nothing fancy, plus adv bridge output.
> Is the driver already using simple display pipeline helpers? I think that
> would be an ideal fit and probably greatly simplifies the code.
> 
> > - minor cleanups
> 
> The patch series looks like it contains the entire development history, or
> at least large chunks of it. That's useful for you, but for upstreaming
> the main focues (especially for smaller drivers) is whether your driver
> uses all the available helpers and integrations correctly. And for that
> it's much easier if the history is cleaned up, and all intermediate steps
> removed.
> 
> I think once that's done I can do a quick pass and drop suggestions for
> cleanup and stuff like that, and then we should (usually at least) be able
> to pull in the driver fairly quickly.
> 
> Another thing to consider is where/how this driver will be maintained.
> Preferred option is as part of drm-misc so that we have redudancy and all
> that in a fairly big group. Works with commit rights, so maybe check out
> some of our docs about that too.
> 
> https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> 
> The committer model comes with a full set of scripts and docs to avoid
> oopsies in maintainership. Generally works really well.

Oh if you have a git branch of this all somewhere I could do a quick
high-level pass and see whether there's anything big.
-Daniel

> 
> Cheers, Daniel
> 
> 
> > 
> > Anitha Chrisanthus (52):
> >   drm/kmb: Add support for KeemBay Display
> >   drm/kmb: Added id to kmb_plane
> >   drm/kmb: Set correct values in the LAYERn_CFG register
> >   drm/kmb: Use biwise operators for register definitions
> >   drm/kmb: Updated kmb_plane_atomic_check
> >   drm/kmb: Initial check-in for Mipi DSI
> >   drm/kmb: Set OUT_FORMAT_CFG register
> >   drm/kmb: Added mipi_dsi_host initialization
> >   drm/kmb: Part 1 of Mipi Tx Initialization
> >   drm/kmb: Part 2 of Mipi Tx Initialization
> >   drm/kmb: Use correct mmio offset from data book
> >   drm/kmb: Part3 of Mipi Tx initialization
> >   drm/kmb: Part4 of Mipi Tx Initialization
> >   drm/kmb: Correct address offsets for mipi registers
> >   drm/kmb: Part5 of Mipi Tx Intitialization
> >   drm/kmb: Part6 of Mipi Tx Initialization
> >   drm/kmb: Part7 of Mipi Tx Initialization
> >   drm/kmb: Part8 of Mipi Tx Initialization
> >   drm/kmb: Added ioremap/iounmap for register access
> >   drm/kmb: Register IRQ for LCD
> >   drm/kmb: IRQ handlers for LCD and mipi dsi
> >   drm/kmb: Set hardcoded values to LCD_VSYNC_START
> >   drm/kmb: Additional register programming to update_plane
> >   drm/kmb: Add ADV7535 bridge
> >   drm/kmb: Display clock enable/disable
> >   drm/kmb: rebase to newer kernel version
> >   drm/kmb: minor name change to match device tree
> >   drm/kmb: Changed MMIO size
> >   drm/kmb: Defer Probe
> >   drm/kmb: call bridge init in the very beginning
> >   drm/kmb: Enable MSS_CAM_CLK_CTRL for LCD and MIPI
> >   drm/kmb: Set MSS_CAM_RSTN_CTRL along with enable
> >   drm/kmb: Mipi DPHY initialization changes
> >   drm/kmb: Fixed driver unload
> >   drm/kmb: Added LCD_TEST config
> >   drm/kmb: Changes for LCD to Mipi
> >   drm/kmb: Update LCD programming to match MIPI
> >   drm/kmb: Changed name of driver to kmb-drm
> >   drm/kmb: Mipi settings from input timings
> >   drm/kmb: Enable LCD interrupts
> >   drm/kmb: Enable LCD interrupts during modeset
> >   drm/kmb: Don’t inadvertantly disable LCD controller
> >   drm/kmb: SWAP R and B LCD Layer order
> >   drm/kmb: Disable ping pong mode
> >   drm/kmb: Do the layer initializations only once
> >   drm/kmb: disable the LCD layer in EOF irq handler
> >   drm/kmb: Initialize uninitialized variables
> >   drm/kmb: Added useful messages in LCD ISR
> >   kmb/drm: Prune unsupported modes
> >   drm/kmb: workaround for dma undeflow issue
> >   drm/kmb: Get System Clock from SCMI
> >   drm/kmb: work around for planar formats
> > 
> > Edmund Dea (7):
> >   drm/kmb: Cleanup probe functions
> >   drm/kmb: Revert dsi_host back to a static variable
> >   drm/kmb: Initialize clocks for clk_msscam, clk_mipi_ecfg, &
> >     clk_mipi_cfg.
> >   drm/kmb: Remove declaration of irq_lcd/irq_mipi
> >   drm/kmb: Enable MIPI TX HS Test Pattern Generation
> >   drm/kmb: Write to LCD_LAYERn_CFG only once
> >   drm/kmb: Cleaned up code
> > 
> >  drivers/gpu/drm/Kconfig         |    2 +
> >  drivers/gpu/drm/Makefile        |    1 +
> >  drivers/gpu/drm/kmb/Kconfig     |   12 +
> >  drivers/gpu/drm/kmb/Makefile    |    2 +
> >  drivers/gpu/drm/kmb/kmb_crtc.c  |  226 +++++
> >  drivers/gpu/drm/kmb/kmb_crtc.h  |   41 +
> >  drivers/gpu/drm/kmb/kmb_drv.c   |  809 ++++++++++++++++
> >  drivers/gpu/drm/kmb/kmb_drv.h   |  176 ++++
> >  drivers/gpu/drm/kmb/kmb_dsi.c   | 1927 +++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/kmb/kmb_dsi.h   |  370 ++++++++
> >  drivers/gpu/drm/kmb/kmb_plane.c |  518 +++++++++++
> >  drivers/gpu/drm/kmb/kmb_plane.h |  124 +++
> >  drivers/gpu/drm/kmb/kmb_regs.h  |  738 +++++++++++++++
> >  13 files changed, 4946 insertions(+)
> >  create mode 100644 drivers/gpu/drm/kmb/Kconfig
> >  create mode 100644 drivers/gpu/drm/kmb/Makefile
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.h
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.c
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.h
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_regs.h
> > 
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel