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