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; > +} _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel