[PATCH RESEND 1/4] drm/nouveau: Merge pre/postclose hooks

[PATCH RESEND 1/4] drm/nouveau: Merge pre/postclose hooks

[Daniel Vetter]

Again no apparent explanation for the split except hysterical raisins.
Merging them also makes it a bit more obviuos what's going on wrt the
runtime pm refdancing.

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 2b6ac24ce690..ec719df619a6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -881,7 +881,7 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv)
 }
 
 static void
-nouveau_drm_preclose(struct drm_device *dev, struct drm_file *fpriv)
+nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
 {
 	struct nouveau_cli *cli = nouveau_cli(fpriv);
 	struct nouveau_drm *drm = nouveau_drm(dev);
@@ -897,12 +897,6 @@ nouveau_drm_preclose(struct drm_device *dev, struct drm_file *fpriv)
 	list_del(&cli->head);
 	mutex_unlock(&drm->client.mutex);
 
-}
-
-static void
-nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
-{
-	struct nouveau_cli *cli = nouveau_cli(fpriv);
 	nouveau_cli_fini(cli);
 	kfree(cli);
 	pm_runtime_mark_last_busy(dev->dev);
@@ -974,7 +968,6 @@ driver_stub = {
 	.load = nouveau_drm_load,
 	.unload = nouveau_drm_unload,
 	.open = nouveau_drm_open,
-	.preclose = nouveau_drm_preclose,
 	.postclose = nouveau_drm_postclose,
 	.lastclose = nouveau_vga_lastclose,
 
-- 
2.11.0

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH RESEND 3/4] drm/exynos: Merge pre/postclose hooks

[Daniel Vetter]

Again no apparent explanation for the split except hysterical raisins.
Merging them also makes it a bit more obviuos what's going on wrt the
runtime pm refdancing.

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 09d3c4c3c858..50294a7bd29d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -82,14 +82,9 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
 	return ret;
 }
 
-static void exynos_drm_preclose(struct drm_device *dev,
-					struct drm_file *file)
-{
-	exynos_drm_subdrv_close(dev, file);
-}
-
 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
 {
+	exynos_drm_subdrv_close(dev, file);
 	kfree(file->driver_priv);
 	file->driver_priv = NULL;
 }
@@ -145,7 +140,6 @@ static struct drm_driver exynos_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
 				  | DRIVER_ATOMIC | DRIVER_RENDER,
 	.open			= exynos_drm_open,
-	.preclose		= exynos_drm_preclose,
 	.lastclose		= exynos_drm_lastclose,
 	.postclose		= exynos_drm_postclose,
 	.gem_free_object_unlocked = exynos_drm_gem_free_object,
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RESEND 3/4] drm/exynos: Merge pre/postclose hooks

[Inki Dae]

Hi Daniel,

2017년 05월 08일 17:26에 Daniel Vetter 이(가) 쓴 글:
> Again no apparent explanation for the split except hysterical raisins.
> Merging them also makes it a bit more obviuos what's going on wrt the
> runtime pm refdancing.

I had requested git-pull,
http://www.spinics.net/lists/dri-devel/msg139194.html

However, Dave had already closed the merge window at -rc6.

As I commented below, most of patches of exynos-drm-next are fixups and cleanup so I will request pull to -fixes.
http://www.spinics.net/lists/dri-devel/msg139214.html

Thanks,
Inki Dae

> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 09d3c4c3c858..50294a7bd29d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -82,14 +82,9 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>  	return ret;
>  }
>  
> -static void exynos_drm_preclose(struct drm_device *dev,
> -					struct drm_file *file)
> -{
> -	exynos_drm_subdrv_close(dev, file);
> -}
> -
>  static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
>  {
> +	exynos_drm_subdrv_close(dev, file);
>  	kfree(file->driver_priv);
>  	file->driver_priv = NULL;
>  }
> @@ -145,7 +140,6 @@ static struct drm_driver exynos_drm_driver = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>  				  | DRIVER_ATOMIC | DRIVER_RENDER,
>  	.open			= exynos_drm_open,
> -	.preclose		= exynos_drm_preclose,
>  	.lastclose		= exynos_drm_lastclose,
>  	.postclose		= exynos_drm_postclose,
>  	.gem_free_object_unlocked = exynos_drm_gem_free_object,
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RESEND 4/4] drm: Nerf the preclose callback for modern drivers

[Daniel Vetter]

With all drivers converted there's only legacy dri1 drivers using it.
Not going to touch those, instead just hide it like we've done with
other dri1 driver hooks like firstopen.

In all this I didn't find any real reason why we'd needed 2 hooks, and
having symmetry between open and close just appeases my OCD better.
Yeah, someone else could do an s/postclose/close/, but that's for
someone who understands cocci. And maybe after this series is reviewed
and landed, to avoid patch-regen churn.

v2: s/last/post/close in the kernel-doc (Sean).

Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_file.c |  8 ++++----
 include/drm/drm_drv.h      | 23 ++---------------------
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 3783b659cd38..caad93dab54b 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -351,9 +351,8 @@ void drm_lastclose(struct drm_device * dev)
  *
  * This function must be used by drivers as their &file_operations.release
  * method. It frees any resources associated with the open file, and calls the
- * &drm_driver.preclose and &drm_driver.lastclose driver callbacks. If this is
- * the last open file for the DRM device also proceeds to call the
- * &drm_driver.lastclose driver callback.
+ * &drm_driver.postclose driver callback. If this is the last open file for the
+ * DRM device also proceeds to call the &drm_driver.lastclose driver callback.
  *
  * RETURNS:
  *
@@ -373,7 +372,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->filelist_mutex);
 
-	if (dev->driver->preclose)
+	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
+	    dev->driver->preclose)
 		dev->driver->preclose(dev, file_priv);
 
 	/* ========================================================
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 53b98321df9b..d0b5f363bfa1 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -104,23 +104,6 @@ struct drm_driver {
 	int (*open) (struct drm_device *, struct drm_file *);
 
 	/**
-	 * @preclose:
-	 *
-	 * One of the driver callbacks when a new &struct drm_file is closed.
-	 * Useful for tearing down driver-private data structures allocated in
-	 * @open like buffer allocators, execution contexts or similar things.
-	 *
-	 * Since the display/modeset side of DRM can only be owned by exactly
-	 * one &struct drm_file (see &drm_file.is_master and &drm_device.master)
-	 * there should never be a need to tear down any modeset related
-	 * resources in this callback. Doing so would be a driver design bug.
-	 *
-	 * FIXME: It is not really clear why there's both @preclose and
-	 * @postclose. Without a really good reason, use @postclose only.
-	 */
-	void (*preclose) (struct drm_device *, struct drm_file *file_priv);
-
-	/**
 	 * @postclose:
 	 *
 	 * One of the driver callbacks when a new &struct drm_file is closed.
@@ -131,9 +114,6 @@ struct drm_driver {
 	 * one &struct drm_file (see &drm_file.is_master and &drm_device.master)
 	 * there should never be a need to tear down any modeset related
 	 * resources in this callback. Doing so would be a driver design bug.
-	 *
-	 * FIXME: It is not really clear why there's both @preclose and
-	 * @postclose. Without a really good reason, use @postclose only.
 	 */
 	void (*postclose) (struct drm_device *, struct drm_file *);
 
@@ -150,7 +130,7 @@ struct drm_driver {
 	 * state changes, e.g. in conjunction with the :ref:`vga_switcheroo`
 	 * infrastructure.
 	 *
-	 * This is called after @preclose and @postclose have been called.
+	 * This is called after @postclose hook has been called.
 	 *
 	 * NOTE:
 	 *
@@ -516,6 +496,7 @@ struct drm_driver {
 	/* List of devices hanging off this driver with stealth attach. */
 	struct list_head legacy_dev_list;
 	int (*firstopen) (struct drm_device *);
+	void (*preclose) (struct drm_device *, struct drm_file *file_priv);
 	int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv);
 	int (*dma_quiescent) (struct drm_device *);
 	int (*context_dtor) (struct drm_device *dev, int context);
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RESEND 2/4] drm/tegra: switch to postclose

[Daniel Vetter]

I didn't spot anything that would require ordering here (well not
anywhere else either), and I'm trying to unify at least modern drivers
on one close hook.

Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Reviewed-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Daniel Vetter <daniel.vetter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/tegra/drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 9a1e34e48f64..51c48a8e00ec 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -892,7 +892,7 @@ static int tegra_drm_context_cleanup(int id, void *p, void *data)
 	return 0;
 }
 
-static void tegra_drm_preclose(struct drm_device *drm, struct drm_file *file)
+static void tegra_drm_postclose(struct drm_device *drm, struct drm_file *file)
 {
 	struct tegra_drm_file *fpriv = file->driver_priv;
 
@@ -960,7 +960,7 @@ static struct drm_driver tegra_drm_driver = {
 	.load = tegra_drm_load,
 	.unload = tegra_drm_unload,
 	.open = tegra_drm_open,
-	.preclose = tegra_drm_preclose,
+	.postclose = tegra_drm_postclose,
 	.lastclose = tegra_drm_lastclose,
 
 #if defined(CONFIG_DEBUG_FS)
-- 
2.11.0

Re: [PATCH RESEND 1/4] drm/nouveau: Merge pre/postclose hooks

[Daniel Vetter]

On Mon, May 08, 2017 at 10:26:30AM +0200, Daniel Vetter wrote:
> Again no apparent explanation for the split except hysterical raisins.
> Merging them also makes it a bit more obviuos what's going on wrt the
> runtime pm refdancing.
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Forgot the cover letter: I'm resending the patches that didn't find an
owner last time around. I'll smash them all into drm-misc later so I can
apply the final patch.

Thanks, Daniel
> ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 2b6ac24ce690..ec719df619a6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -881,7 +881,7 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv)
>  }
>  
>  static void
> -nouveau_drm_preclose(struct drm_device *dev, struct drm_file *fpriv)
> +nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
>  {
>  	struct nouveau_cli *cli = nouveau_cli(fpriv);
>  	struct nouveau_drm *drm = nouveau_drm(dev);
> @@ -897,12 +897,6 @@ nouveau_drm_preclose(struct drm_device *dev, struct drm_file *fpriv)
>  	list_del(&cli->head);
>  	mutex_unlock(&drm->client.mutex);
>  
> -}
> -
> -static void
> -nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
> -{
> -	struct nouveau_cli *cli = nouveau_cli(fpriv);
>  	nouveau_cli_fini(cli);
>  	kfree(cli);
>  	pm_runtime_mark_last_busy(dev->dev);
> @@ -974,7 +968,6 @@ driver_stub = {
>  	.load = nouveau_drm_load,
>  	.unload = nouveau_drm_unload,
>  	.open = nouveau_drm_open,
> -	.preclose = nouveau_drm_preclose,
>  	.postclose = nouveau_drm_postclose,
>  	.lastclose = nouveau_vga_lastclose,
>  
> -- 
> 2.11.0
> 

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