Hi Daniel,
Am Mittwoch, den 03.02.2016, 01:12 +0800 schrieb Daniel Kurtz:
[...]
> > +int mtk_drm_crtc_create(struct drm_device *drm_dev,
> > + const enum mtk_ddp_comp_id *path, unsigned int path_len)
> > +{
[...]
> > + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > + enum mtk_ddp_comp_id comp_id = path[i];
> > + struct mtk_ddp_comp *comp;
I'll check for (priv->comp_node[comp_id] != NULL) here.
> > + 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.
^^
[...]
> > 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
[...]
> > +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()?
This is deprecated, I had used it that way until v4.
Can we just drop the set_busid callback?
regards
Philipp