[PATCH v9 02/14] drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.
Daniel Kurtz
djkurtz at chromium.org
Tue Feb 2 09:12:12 PST 2016
Hi Philipp,
Sorry for the previous HTML email. Trying again...
Below are some more comments from my observations today trying to configure
Mediatek DRM to use just the MIPI/DSI path (no HDMI).
On Tue, Jan 12, 2016 at 11:15 PM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
> From: CK Hu <ck.hu at mediatek.com>
>
> This patch adds an initial DRM driver for the Mediatek MT8173 DISP
> subsystem. It currently supports two fixed output streams from the
> OVL0/OVL1 sources to the DSI0/DPI0 sinks, respectively.
>
> Signed-off-by: CK Hu <ck.hu at mediatek.com>
> Signed-off-by: YT Shen <yt.shen at mediatek.com>
> Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
> Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
[snip]
> +int mtk_drm_crtc_create(struct drm_device *drm_dev,
> + const enum mtk_ddp_comp_id *path, unsigned int path_len)
> +{
> + struct mtk_drm_private *priv = drm_dev->dev_private;
> + struct device *dev = drm_dev->dev;
> + struct mtk_drm_crtc *mtk_crtc;
> + enum drm_plane_type type;
> + unsigned int zpos;
> + int pipe = priv->num_pipes;
> + int ret;
> + int i;
> +
> + mtk_crtc = devm_kzalloc(dev, sizeof(*mtk_crtc), GFP_KERNEL);
> + if (!mtk_crtc)
> + return -ENOMEM;
> +
> + mtk_crtc->config_regs = priv->config_regs;
> + mtk_crtc->ddp_comp_nr = path_len;
> + mtk_crtc->ddp_comp = devm_kmalloc_array(dev, mtk_crtc->ddp_comp_nr,
> + sizeof(*mtk_crtc->ddp_comp),
> + GFP_KERNEL);
> +
> + mtk_crtc->mutex = mtk_disp_mutex_get(priv->mutex_dev, pipe);
> + if (IS_ERR(mtk_crtc->mutex)) {
> + ret = PTR_ERR(mtk_crtc->mutex);
> + dev_err(dev, "Failed to get mutex: %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> + enum mtk_ddp_comp_id comp_id = path[i];
> + struct mtk_ddp_comp *comp;
> +
> + comp = priv->ddp_comp[comp_id];
> + if (!comp) {
> + dev_err(dev, "Component %s not initialized\n",
> + priv->comp_node[comp_id]->full_name);
If one of the components is disabled in .dtsi, then its
priv->comp_node[comp_id] will be NULL here, and trying to full_name will OOPS.
> + ret = -ENODEV;
> + goto unprepare;
> + }
> +
> + ret = clk_prepare(comp->clk);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare clock for component %s: %d\n",
> + priv->comp_node[comp_id]->full_name, ret);
> + goto unprepare;
> + }
> +
> + mtk_crtc->ddp_comp[i] = comp;
> + }
> +
> + for (zpos = 0; zpos < OVL_LAYER_NR; zpos++) {
> + type = (zpos == 0) ? DRM_PLANE_TYPE_PRIMARY :
> + (zpos == 1) ? DRM_PLANE_TYPE_CURSOR :
> + DRM_PLANE_TYPE_OVERLAY;
> + ret = mtk_plane_init(drm_dev, &mtk_crtc->planes[zpos],
> + BIT(pipe), type, zpos);
> + if (ret)
> + goto unprepare;
> + }
> +
> + ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, &mtk_crtc->planes[0].base,
> + &mtk_crtc->planes[1].base, pipe);
> + if (ret < 0)
> + goto unprepare;
> +
> + priv->crtc[pipe] = &mtk_crtc->base;
> + priv->num_pipes++;
> +
> + return 0;
> +
> +unprepare:
> + while (--i >= 0)
> + clk_unprepare(mtk_crtc->ddp_comp[i]->clk);
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> new file mode 100644
> index 0000000..9db22b4
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
[snip]
> +static struct drm_driver mtk_drm_driver = {
> + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
> + DRIVER_ATOMIC,
> + .unload = mtk_drm_unload,
> + .set_busid = drm_platform_set_busid,
I think using drm_platform_set_busid() as our .set_busid may cause an OOPs if
userspace does DRM_IOCTL_SET_VERSION.
drm_setversion() will calls drm_set_busid(), which calls
dev->driver->set_busid()
drm_platform_set_busid() accesses dev->platformdev->id.
However, dev->platformdev is only set by:
drm_platform_init()->drm_get_platform_dev()
And, since mtk_drm_bind() does the drm_dev_alloc() /
drm_dev_register() itself instead
of calling drm_get_platform_dev(), so dev->platformdev will still be NULL.
So, why don't we call drm_platform_init() instead and implement a .load callback
to do mtk_drm_kms_init()?
> +
> + .get_vblank_counter = drm_vblank_count,
> + .enable_vblank = mtk_drm_crtc_enable_vblank,
> + .disable_vblank = mtk_drm_crtc_disable_vblank,
> +
> + .gem_free_object = mtk_drm_gem_free_object,
> + .gem_vm_ops = &mtk_drm_gem_vm_ops,
> + .dumb_create = mtk_drm_gem_dumb_create,
> + .dumb_map_offset = mtk_drm_gem_dumb_map_offset,
> + .dumb_destroy = drm_gem_dumb_destroy,
> +
> + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> + .gem_prime_export = drm_gem_prime_export,
> + .gem_prime_import = drm_gem_prime_import,
> + .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> + .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> + .fops = &mtk_drm_fops,
> +
> + .name = DRIVER_NAME,
> + .desc = DRIVER_DESC,
> + .date = DRIVER_DATE,
> + .major = DRIVER_MAJOR,
> + .minor = DRIVER_MINOR,
> +};
> +
> +static int compare_of(struct device *dev, void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +static int mtk_drm_bind(struct device *dev)
> +{
> + struct mtk_drm_private *private = dev_get_drvdata(dev);
> + struct drm_device *drm;
> + int ret;
> +
> + drm = drm_dev_alloc(&mtk_drm_driver, dev);
> + if (!drm)
> + return -ENOMEM;
> +
> + drm_dev_set_unique(drm, dev_name(dev));
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret < 0)
> + goto err_free;
> +
> + drm->dev_private = private;
> + private->drm = drm;
> +
> + ret = mtk_drm_kms_init(drm);
> + if (ret < 0)
> + goto err_unregister;
> +
> + return 0;
> +
> +err_unregister:
> + drm_dev_unregister(drm);
> +err_free:
> + drm_dev_unref(drm);
> + return ret;
> +}
More information about the Linux-mediatek
mailing list