All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Liviu Dudau <liviu.dudau@arm.com>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Russell King <rmk+kernel@arm.linux.org.uk>
Subject: Re: [PATCH] drm: Nuke ->vblank_disable_allowed
Date: Fri, 20 May 2016 01:02:58 +0300	[thread overview]
Message-ID: <2096897.4O8cYlVSp2@avalon> (raw)
In-Reply-To: <1463603397-23551-1-git-send-email-daniel.vetter@ffwll.ch>

Hi Daniel,

Thank you for the patch.

On Wednesday 18 May 2016 22:29:57 Daniel Vetter wrote:
> This was added in
> 
> commit 0a3e67a4caac273a3bfc4ced3da364830b1ab241
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Tue Sep 30 12:14:26 2008 -0700
> 
>     drm: Rework vblank-wait handling to allow interrupt reduction.
> 
> to stay backwards-compatible with old UMS code that didn't even tell
> the kernel when it did a modeset, so that the kernel could
> save/restore vblank counters. At worst this means vblanks will be
> somewhat funky on a setup that very likely no one still runs.
> 
> So let's just nuke it.
> 
> Plan B would be to set it unconditionally in drm_vblank_init for kms
> drivers, instead of in each driver separately. So if this patch breaks
> anything please only restore the hunks in drmP.h and drm_irq.c, plus
> add a check for DRIVER_MODESET in drm_vblank_init.
> 
> Stumbled over this in a discussion on irc with Chris.
> 
> v2: Remove leftover debug gunk from psr hacking (Alex).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and for rcar-du-drm,

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c     | 1 -
>  drivers/gpu/drm/arm/hdlcd_drv.c             | 1 -
>  drivers/gpu/drm/armada/armada_drv.c         | 1 -
>  drivers/gpu/drm/drm_irq.c                   | 6 ------
>  drivers/gpu/drm/exynos/exynos_drm_drv.c     | 7 -------
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 1 -
>  drivers/gpu/drm/gma500/psb_drv.c            | 1 -
>  drivers/gpu/drm/i915/i915_dma.c             | 3 ---
>  drivers/gpu/drm/imx/imx-drm-core.c          | 7 -------
>  drivers/gpu/drm/radeon/radeon_irq_kms.c     | 1 -
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 7 -------
>  drivers/gpu/drm/tegra/drm.c                 | 1 -
>  drivers/gpu/drm/vc4/vc4_kms.c               | 2 --
>  include/drm/drmP.h                          | 8 --------
>  14 files changed, 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 9266c7b69808..835a3fa8d8df
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -219,7 +219,6 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>  	if (r) {
>  		return r;
>  	}
> -	adev->ddev->vblank_disable_allowed = true;
> 
>  	/* enable msi */
>  	adev->irq.msi_enabled = false;
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c
> b/drivers/gpu/drm/arm/hdlcd_drv.c index 2112f0b105e3..4f909378d581 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -379,7 +379,6 @@ static int hdlcd_drm_bind(struct device *dev)
>  		DRM_ERROR("failed to initialise vblank\n");
>  		goto err_vblank;
>  	}
> -	drm->vblank_disable_allowed = true;
> 
>  	drm_mode_config_reset(drm);
>  	drm_kms_helper_poll_init(drm);
> diff --git a/drivers/gpu/drm/armada/armada_drv.c
> b/drivers/gpu/drm/armada/armada_drv.c index 531fcb946346..cb21c0b6374a
> 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -113,7 +113,6 @@ static int armada_drm_load(struct drm_device *dev,
> unsigned long flags) goto err_comp;
> 
>  	dev->irq_enabled = true;
> -	dev->vblank_disable_allowed = 1;
> 
>  	ret = armada_fbdev_init(dev);
>  	if (ret)
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index d1b5fc20b2f8..1a0ae89087e8 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -348,9 +348,6 @@ static void vblank_disable_fn(unsigned long arg)
>  	unsigned int pipe = vblank->pipe;
>  	unsigned long irqflags;
> 
> -	if (!dev->vblank_disable_allowed)
> -		return;
> -
>  	spin_lock_irqsave(&dev->vbl_lock, irqflags);
>  	if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) {
>  		DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
> @@ -437,8 +434,6 @@ int drm_vblank_init(struct drm_device *dev, unsigned int
> num_crtcs) "get_vblank_timestamp == NULL\n");
>  	}
> 
> -	dev->vblank_disable_allowed = false;
> -
>  	return 0;
> 
>  err:
> @@ -1555,7 +1550,6 @@ static void drm_legacy_vblank_post_modeset(struct
> drm_device *dev,
> 
>  	if (vblank->inmodeset) {
>  		spin_lock_irqsave(&dev->vbl_lock, irqflags);
> -		dev->vblank_disable_allowed = true;
>  		drm_reset_vblank_timestamp(dev, pipe);
>  		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 21c719e8e02b..2dd820e23b0c
> 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -212,13 +212,6 @@ static int exynos_drm_load(struct drm_device *dev,
> unsigned long flags) */
>  	dev->irq_enabled = true;
> 
> -	/*
> -	 * with vblank_disable_allowed = true, vblank interrupt will be disabled
> -	 * by drm timer once a current process gives up ownership of
> -	 * vblank event.(after drm_vblank_put function is called)
> -	 */
> -	dev->vblank_disable_allowed = true;
> -
>  	/* init kms poll for handling hpd */
>  	drm_kms_helper_poll_init(dev);
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index
> 2e58dfd74ae1..33727d5d826a 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -80,7 +80,6 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned
> long flags) dev_err(dev->dev, "failed to initialize vblank\n");
>  		goto done;
>  	}
> -	dev->vblank_disable_allowed = true;
> 
>  	ret = fsl_dcu_drm_irq_init(dev);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c
> b/drivers/gpu/drm/gma500/psb_drv.c index 4e1c6850520e..82b8ce418b27 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -374,7 +374,6 @@ static int psb_driver_load(struct drm_device *dev,
> unsigned long flags)
> 
>  	drm_irq_install(dev, dev->pdev->irq);
> 
> -	dev->vblank_disable_allowed = true;
>  	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
>  	dev->driver->get_vblank_counter = psb_get_vblank_counter;
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c index a8c79f6512a4..fd06bff216ff 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -515,9 +515,6 @@ static int i915_load_modeset_init(struct drm_device
> *dev)
> 
>  	intel_modeset_gem_init(dev);
> 
> -	/* Always safe in the mode setting case. */
> -	/* FIXME: do pre/post-mode set stuff in core KMS code */
> -	dev->vblank_disable_allowed = true;
>  	if (INTEL_INFO(dev)->num_pipes == 0)
>  		return 0;
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
> b/drivers/gpu/drm/imx/imx-drm-core.c index 2453fb1c68a7..1080019e7b17
> 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -252,13 +252,6 @@ static int imx_drm_driver_load(struct drm_device *drm,
> unsigned long flags) if (ret)
>  		goto err_kms;
> 
> -	/*
> -	 * with vblank_disable_allowed = true, vblank interrupt will be
> -	 * disabled by drm timer once a current process gives up ownership
> -	 * of vblank event. (after drm_vblank_put function is called)
> -	 */
> -	drm->vblank_disable_allowed = true;
> -
>  	platform_set_drvdata(drm->platformdev, drm);
> 
>  	/* Now try and bind all our sub-components */
> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> b/drivers/gpu/drm/radeon/radeon_irq_kms.c index 1e9304d1c88f..c084cadcbf21
> 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -291,7 +291,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>  	if (r) {
>  		return r;
>  	}
> -	rdev->ddev->vblank_disable_allowed = true;
> 
>  	/* enable msi */
>  	rdev->msi_enabled = 0;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index
> ced1e5f93cda..09a4d429c0f0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -217,13 +217,6 @@ static int rockchip_drm_load(struct drm_device
> *drm_dev, unsigned long flags) if (ret)
>  		goto err_kms_helper_poll_fini;
> 
> -	/*
> -	 * with vblank_disable_allowed = true, vblank interrupt will be disabled
> -	 * by drm timer once a current process gives up ownership of
> -	 * vblank event.(after drm_vblank_put function is called)
> -	 */
> -	drm_dev->vblank_disable_allowed = true;
> -
>  	drm_mode_config_reset(drm_dev);
> 
>  	ret = rockchip_drm_fbdev_init(drm_dev);
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 679905544181..b59c3bf0df44 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -180,7 +180,6 @@ static int tegra_drm_load(struct drm_device *drm,
> unsigned long flags)
> 
>  	/* syncpoints are used for full 32-bit hardware VBLANK counters */
>  	drm->max_vblank_count = 0xffffffff;
> -	drm->vblank_disable_allowed = true;
> 
>  	err = drm_vblank_init(drm, drm->mode_config.num_crtc);
>  	if (err < 0)
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index d423ba10239a..cb37751bc99f 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -207,8 +207,6 @@ int vc4_kms_load(struct drm_device *dev)
>  	dev->mode_config.preferred_depth = 24;
>  	dev->mode_config.async_page_flip = true;
> 
> -	dev->vblank_disable_allowed = true;
> -
>  	drm_mode_config_reset(dev);
> 
>  	vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index f0d1450d4ea5..30dc7d8c403f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -819,14 +819,6 @@ struct drm_device {
>  	int irq;
> 
>  	/*
> -	 * At load time, disabling the vblank interrupt won't be allowed since
> -	 * old clients may not call the modeset ioctl and therefore misbehave.
> -	 * Once the modeset ioctl *has* been called though, we can safely
> -	 * disable them when unused.
> -	 */
> -	bool vblank_disable_allowed;
> -
> -	/*
>  	 * If true, vblank interrupt will be disabled immediately when the
>  	 * refcount drops to zero, as opposed to via the vblank disable
>  	 * timer.

-- 
Regards,

Laurent Pinchart

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

      parent reply	other threads:[~2016-05-19 22:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-18 19:47 [PATCH] drm: Nuke ->vblank_disable_allowed Daniel Vetter
2016-05-18 20:02 ` Alex Deucher
2016-05-18 20:29 ` Daniel Vetter
2016-05-18 21:54   ` Alex Deucher
2016-05-19  8:02   ` Liviu Dudau
2016-05-19 22:02   ` Laurent Pinchart [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2096897.4O8cYlVSp2@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=alexander.deucher@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=liviu.dudau@arm.com \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.