Re: [PATCH v14 2/8] drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.

From: Daniel Vetter <daniel@ffwll.ch>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Emil Velikov <emil.l.velikov@gmail.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Jie Qiu <jie.qiu@mediatek.com>,
	Cawa Cheng <cawa.cheng@mediatek.com>,
	Mao Huang <littlecvr@chromium.org>,
	Yingjoe Chen <yingjoe.chen@mediatek.com>,
	devicetree@vger.kernel.org, Jitao Shi <jitao.shi@mediatek.com>,
	kernel@pengutronix.de, Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	YT Shen <yt.shen@mediatek.com>, Rob Herring <robh+dt@kernel.org>,
	linux-mediatek@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Paul Bolle <pebolle@tiscali.nl>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Tomasz Figa <tfiga@chromium.org>,
	Kumar Gala <galak@codeaurora.org>
Subject: Re: [PATCH v14 2/8] drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.
Date: Wed, 27 Apr 2016 12:57:33 +0200	[thread overview]
Message-ID: <20160427105733.GZ2558@phenom.ffwll.local> (raw)
In-Reply-To: <1461752945.4044.25.camel@pengutronix.de>

On Wed, Apr 27, 2016 at 12:29:05PM +0200, Philipp Zabel wrote:
> Hi Daniel,
> 
> Am Dienstag, den 26.04.2016, 10:48 +0200 schrieb Daniel Vetter:
> [...]
> > > +static int mtk_drm_unload(struct drm_device *drm)
> > > +{
> > > +	mtk_drm_kms_deinit(drm);
> > > +	drm->dev_private = NULL;
> > > +
> > > +	return 0;
> > > +}
> > 
> > Like load, the unload hook is deprecated. And in drm-next we now have some
> > nice helpers to make not using an unload hook easier. Please refactor per
> > existing examples and get rid of your unload hook.
> 
> Thanks, does this change look about right:

yup, lgtm, ack.
-Daniel

> 
> --------8<--------
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -222,14 +222,6 @@ static void mtk_drm_kms_deinit(struct drm_device
> *drm)
>  	drm_mode_config_cleanup(drm);
>  }
>  
> -static int mtk_drm_unload(struct drm_device *drm)
> -{
> -	mtk_drm_kms_deinit(drm);
> -	drm->dev_private = NULL;
> -
> -	return 0;
> -}
> -
>  static const struct file_operations mtk_drm_fops = {
>  	.owner = THIS_MODULE,
>  	.open = drm_open,
> @@ -246,7 +238,6 @@ static const struct file_operations mtk_drm_fops = {
>  static struct drm_driver mtk_drm_driver = {
>  	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>  			   DRIVER_ATOMIC,
> -	.unload = mtk_drm_unload,
>  
>  	.get_vblank_counter = drm_vblank_count,
>  	.enable_vblank = mtk_drm_crtc_enable_vblank,
> @@ -455,8 +446,14 @@ err_node:
>  static int mtk_drm_remove(struct platform_device *pdev)
>  {
>  	struct mtk_drm_private *private = platform_get_drvdata(pdev);
> +	struct drm_device *drm = private->drm;
>  	int i;
>  
> +	drm_connector_unregister_all(drm);
> +	drm_dev_unregister(drm);
> +	mtk_drm_kms_deinit(drm);
> +	drm_dev_unref(drm);
> +
>  	component_master_del(&pdev->dev, &mtk_drm_ops);
>  	pm_runtime_disable(&pdev->dev);
>  	of_node_put(private->mutex_node);
> -------->8--------
> 
> > > +static const struct vm_operations_struct mtk_drm_gem_vm_ops = {
> > > +	.open = drm_gem_vm_open,
> > > +	.close = drm_gem_vm_close,
> > > +};
> > 
> > Drive-by review I just spotted: Please use drm_gem_cma_vm_ops instead of
> > rolling your own.
> 
> Good catch, will remove.
> 
> regards
> Philipp
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel