Daniel Vetter daniel@ffwll.ch writes:
On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote:
From: Tom Cooksey tom.cooksey@arm.com
This is a modesetting driver for the pl111 CLCD display controller found on various ARM platforms such as the Versatile Express. The driver has only been tested on the bcm911360_entphn platform so far, with PRIME-based buffer sharing between vc4 and clcd.
It reuses the existing devicetree binding, while not using quite as many of its properties as the fbdev driver does (those are left for future work).
v2: Nearly complete rewrite by anholt, cutting 2/3 of the code thanks to DRM core's excellent new helpers.
Signed-off-by: Tom Cooksey tom.cooksey@arm.com Signed-off-by: Eric Anholt eric@anholt.net
Looks pretty. A few things below, but nothing big. I'd say if the "how generic do we want this to be for now" question is resolved it's ready to go in.
If you want this in drm-misc (imo fine, you're already there so doesn't really extend the scope of the experiment), then please also add a MAINTAINERS entry with the drm-misc git repo and yourself as reviewer.
Will do.
diff --git a/drivers/gpu/drm/pl111/pl111_connector.c b/drivers/gpu/drm/pl111/pl111_connector.c new file mode 100644 index 000000000000..9811d1eadb63 --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_connector.c @@ -0,0 +1,127 @@ +/*
- (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved.
- Parts of this file were based on sources as follows:
- Copyright (c) 2006-2008 Intel Corporation
- Copyright (c) 2007 Dave Airlie airlied@linux.ie
- Copyright (C) 2011 Texas Instruments
- This program is free software and is provided to you under the terms of the
- GNU General Public License version 2 as published by the Free Software
- Foundation, and any use by you of this program is subject to the terms of
- such GNU licence.
- */
+/**
- pl111_drm_connector.c
- Implementation of the connector functions for PL111 DRM
- */
+#include <linux/amba/clcd-regs.h> +#include <linux/version.h> +#include <linux/shmem_fs.h> +#include <linux/dma-buf.h>
+#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_of.h> +#include <drm/drm_panel.h>
+#include "pl111_drm.h"
+static void pl111_connector_destroy(struct drm_connector *connector) +{
- struct pl111_drm_connector *pl111_connector =
to_pl111_connector(connector);- if (pl111_connector->panel)
drm_panel_detach(pl111_connector->panel);- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
+}
+static enum drm_connector_status pl111_connector_detect(struct drm_connector
*connector, bool force)+{
- struct pl111_drm_connector *pl111_connector =
to_pl111_connector(connector);- return (pl111_connector->panel ?
connector_status_connected :connector_status_disconnected);+}
+static int pl111_connector_helper_get_modes(struct drm_connector *connector) +{
- struct pl111_drm_connector *pl111_connector =
to_pl111_connector(connector);- if (!pl111_connector->panel)
return 0;- return drm_panel_get_modes(pl111_connector->panel);
+}
Probably the umptenth time I've seen this :(
One option I think would work well is if we have a generic "wrap a drm_panel into a drm_bridge" driver and just glue that in with an of helper as the last element in the enc/transcoder. Laurent has that practically written already, but he insist in calling it the lvds bridge, because it's for a 100% dummy lvds transcoder.
But since it's 100% dummy it's indistinguishable from pure sw abstraction/impendence mismatch helper.
Anyway, just an idea, not going to ask you to do this, but if drm_panel takes of like crazy we'll probably want this.
It seems like a generalization of lvds_encoder to wrap a panel as a connector would be useful. I think I'd like to look at that as a follow-on change.
+static int pl111_modeset_init(struct drm_device *dev) +{
- struct drm_mode_config *mode_config;
- struct pl111_drm_dev_private *priv = dev->dev_private;
- int ret = 0;
- if (!priv)
return -EINVAL;- drm_mode_config_init(dev);
- mode_config = &dev->mode_config;
- mode_config->funcs = &mode_config_funcs;
- mode_config->min_width = 1;
- mode_config->max_width = 1024;
- mode_config->min_height = 1;
- mode_config->max_height = 768;
- ret = pl111_primary_plane_init(dev);
- if (ret != 0) {
dev_err(dev->dev, "Failed to init primary plane\n");goto out_config;- }
I assume this display IP has a pile of planes? Otherwise the simple pipe helpers look like a perfect fit.
Only two, actually. And the other (cursor) is a 64x64 monochrome with mask, so I'm not sure it's going to make sense to ever expose it. I think I'll give the simple helper a try before resubmitting.
+static const struct file_operations drm_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .release = drm_release,
- .unlocked_ioctl = drm_ioctl,
- .mmap = drm_gem_cma_mmap,
- .poll = drm_poll,
- .read = drm_read,
+};
Very recent, but DEFINE_DRM_GEM_CMA_FOPS.
Will do.