Hi Rob, As I wanted to re-use your nxp-tda998x driver for the Marvell Dove SoC, I had a look at your IT LCD driver. Comments below.
On Tue, 22 Jan 2013 16:36:22 -0600 Rob Clark <[email protected]> wrote: > A simple DRM/KMS driver for the TI LCD Controller found in various > smaller TI parts (AM33xx, OMAPL138, etc). This driver uses the > CMA helpers. Currently only the TFP410 DVI encoder is supported > (tested with beaglebone + DVI cape). There are also various LCD > displays, for which support can be added (as I get hw to test on), > and an external i2c HDMI encoder found on some boards. > > The display controller supports a single CRTC. And the encoder+ > connector are split out into sub-devices. Depending on which LCD > or external encoder is actually present, the appropriate output > module(s) will be loaded. > > v1: original > v2: fix fb refcnting and few other cleanups > v3: get +/- vsync/hsync from timings rather than panel-info, add > option DT max-bandwidth field so driver doesn't attempt to > pick a display mode with too high memory bandwidth, and other > small cleanups > > Signed-off-by: Rob Clark <[email protected]> > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/tilcdc/Kconfig | 10 + > drivers/gpu/drm/tilcdc/Makefile | 8 + > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 597 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 605 > +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 159 +++++++++ > drivers/gpu/drm/tilcdc/tilcdc_regs.h | 154 +++++++++ > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 423 +++++++++++++++++++++++ > drivers/gpu/drm/tilcdc/tilcdc_tfp410.h | 26 ++ > 10 files changed, 1985 insertions(+) > create mode 100644 drivers/gpu/drm/tilcdc/Kconfig > create mode 100644 drivers/gpu/drm/tilcdc/Makefile > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_crtc.c > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_drv.c > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_drv.h > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_regs.h > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c > create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_tfp410.h [snip] > diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig > new file mode 100644 > index 0000000..ee9b592 > --- /dev/null > +++ b/drivers/gpu/drm/tilcdc/Kconfig > @@ -0,0 +1,10 @@ > +config DRM_TILCDC > + tristate "DRM Support for TI LCDC Display Controller" > + depends on DRM && OF > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_GEM_CMA_HELPER > + help > + Choose this option if you have an TI SoC with LCDC display > + controller, for example AM33xx in beagle-bone, DA8xx, or > + OMAP-L1xx. This driver replaces the FB_DA8XX fbdev driver. > diff --git a/drivers/gpu/drm/tilcdc/Makefile b/drivers/gpu/drm/tilcdc/Makefile > new file mode 100644 > index 0000000..1359cc2 > --- /dev/null > +++ b/drivers/gpu/drm/tilcdc/Makefile > @@ -0,0 +1,8 @@ > +ccflags-y := -Iinclude/drm -Werror > + > +tilcdc-y := \ > + tilcdc_crtc.o \ > + tilcdc_tfp410.o \ > + tilcdc_drv.o As I understand, this means that the 3 objects will go into the resident kernel. I though that the device tree was created for Linux distributors who might generate generic ARM kernels, the choice of the drivers being done according the local device tree. >From this point of vue, it would be nicer to have 2 separate modules: tilcdc and tfp410, tilcdc_crtc being included in one of these ones (I did not look deep enough at the code to know which). [snip] > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > new file mode 100644 > index 0000000..cf1fddc > --- /dev/null > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > @@ -0,0 +1,605 @@ [snip] > +static struct drm_driver tilcdc_driver = { > + .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET, > + .load = tilcdc_load, > + .unload = tilcdc_unload, > + .preclose = tilcdc_preclose, > + .lastclose = tilcdc_lastclose, > + .irq_handler = tilcdc_irq, > + .irq_preinstall = tilcdc_irq_preinstall, > + .irq_postinstall = tilcdc_irq_postinstall, > + .irq_uninstall = tilcdc_irq_uninstall, > + .get_vblank_counter = drm_vblank_count, > + .enable_vblank = tilcdc_enable_vblank, > + .disable_vblank = tilcdc_disable_vblank, > + .gem_free_object = drm_gem_cma_free_object, > + .gem_vm_ops = &drm_gem_cma_vm_ops, > + .dumb_create = drm_gem_cma_dumb_create, > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > + .dumb_destroy = drm_gem_cma_dumb_destroy, > +#ifdef CONFIG_DEBUG_FS > + .debugfs_init = tilcdc_debugfs_init, > + .debugfs_cleanup = tilcdc_debugfs_cleanup, > +#endif > + .fops = &fops, > + .name = "tilcdc", > + .desc = "TI LCD Controller DRM", > + .date = "20121205", > + .major = 1, > + .minor = 0, > +}; > + > +/* > + * Power management: > + */ > + > +#if CONFIG_PM_SLEEP Should be: #ifdef CONFIG_PM_SLEEP [snip] > +static struct of_device_id tilcdc_of_match[] = { > + { .compatible = "ti,am33xx-tilcdc", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, tilcdc_of_match); > + > +static struct platform_driver tilcdc_platform_driver = { > + .probe = tilcdc_pdev_probe, > + .remove = tilcdc_pdev_remove, > + .driver = { > + .owner = THIS_MODULE, > + .name = "tilcdc", > + .pm = &tilcdc_pm_ops, > + .of_match_table = tilcdc_of_match, > + }, > +}; > + > +static int __init tilcdc_drm_init(void) > +{ > + DBG("init"); > + tilcdc_tfp410_init(); This function may be called twice if "tilcdc,tfp410" is in the DT. > + return platform_driver_register(&tilcdc_platform_driver); > +} > + > +static void __exit tilcdc_drm_fini(void) > +{ > + DBG("fini"); > + tilcdc_tfp410_fini(); > + platform_driver_unregister(&tilcdc_platform_driver); > +} > + > +module_init(tilcdc_drm_init); > +module_exit(tilcdc_drm_fini); > + > +MODULE_AUTHOR("Rob Clark <[email protected]"); > +MODULE_DESCRIPTION("TI LCD Controller DRM Driver"); > +MODULE_LICENSE("GPL"); -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
