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@xxxxxxxxxxxxxx> wrote:
> From: CK Hu <ck.hu@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: YT Shen <yt.shen@xxxxxxxxxxxx>
> Signed-off-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
[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;
> +}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
