Hi Rob,
On Fri, 26 Sep 2014 17:10:49 -0400 Rob Clark robdclark@gmail.com wrote:
On Mon, Sep 8, 2014 at 4:43 AM, Boris BREZILLON boris.brezillon@free-electrons.com wrote:
The Atmel HLCDC (HLCD Controller) IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5 family or sama5d3 family) provides a display controller device.
This display controller supports at least one primary plane and might provide several overlays and an hardware cursor depending on the IP version.
At the moment, this driver only implements an RGB connector to interface with LCD panels, but support for other kind of external devices might be added later.
Signed-off-by: Boris BREZILLON boris.brezillon@free-electrons.com Tested-by: Ludovic Desroches ludovic.desroches@atmel.com
A few small comments below, but with those addressed it has my
Reviewed-by: Rob Clark robdclark@gmail.com
Thanks for your review.
drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/atmel-hlcdc/Kconfig | 13 + drivers/gpu/drm/atmel-hlcdc/Makefile | 7 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 286 ++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 488 +++++++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 224 ++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 656 ++++++++++++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h | 403 +++++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 476 +++++++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 836 +++++++++++++++++++++++ 11 files changed, 3392 insertions(+) create mode 100644 drivers/gpu/drm/atmel-hlcdc/Kconfig create mode 100644 drivers/gpu/drm/atmel-hlcdc/Makefile create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
[snip]
+/**
- Atmel HLCDC plane rotation enum
- TODO: export DRM_ROTATE_XX macros defined by omap driver and use them
- instead of defining this enum.
- */
+enum atmel_hlcdc_plane_rotation {
ATMEL_HLCDC_PLANE_NO_ROTATION,ATMEL_HLCDC_PLANE_90DEG_ROTATION,ATMEL_HLCDC_PLANE_180DEG_ROTATION,ATMEL_HLCDC_PLANE_270DEG_ROTATION,+};
DRM_ROTATE_* are already in drm_crtc.h
[snip]
Yep, I changed that, but won't be able to test until next week.
+static int atmel_hlcdc_rgb_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)+{
return MODE_OK;+}
your _mode_valid() should perhaps somehow check the constraints in atmel_hlcdc_crtc_mode_set()? This way invalid modes get filtered out earlier..
I'm not sure, the test done in atmel_hlcdc_crtc_mode_set are not connector related, but rather imposed by the display controller limitations. Anyway, let me know if you still think I should move those tests in the connector mode_valid implementation.
[snip]
+static struct atmel_hlcdc_plane_properties * +atmel_hlcdc_plane_create_properties(struct drm_device *dev) +{
struct atmel_hlcdc_plane_properties *props;const struct drm_prop_enum_list rotations[] = {{ ATMEL_HLCDC_PLANE_NO_ROTATION, "rotate-0" },{ ATMEL_HLCDC_PLANE_90DEG_ROTATION, "rotate-90" },{ ATMEL_HLCDC_PLANE_180DEG_ROTATION, "rotate-180" },{ ATMEL_HLCDC_PLANE_270DEG_ROTATION, "rotate-270" },};you could just use drm_mode_create_rotation_property() now
Yep, I changed that too. Thanks for the tip.
Best Regards,
Boris