From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <robdclark@gmail.com>
Cc: Rob Clark <robdclark@chromium.org>,
open list <linux-kernel@vger.kernel.org>,
dri-devel@lists.freedesktop.org,
Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH] drm/vblank: Simplify drm_dev_has_vblank()
Date: Tue, 4 Apr 2023 22:46:00 +0200 [thread overview]
Message-ID: <ZCyMiERvNHTP45o8@phenom.ffwll.local> (raw)
In-Reply-To: <20230403160735.1211468-1-robdclark@gmail.com>
On Mon, Apr 03, 2023 at 09:07:35AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> What does vblank have to do with num_crtcs? Well, this was technically
> correct, but you'd have to go look at where num_crtcs is initialized to
> understand why. Lets just replace it with the simpler and more obvious
> check.
If you want to fix this, then I think the right fix is to rename num_crtcs
to be something like num_vblank_crtcs. It's a historical accident back
when vblanks without kms was a thing.
Plan B is someone gets really busy and fixes up the entire vblank mess and
moves it into drm_crtc struct. Now that the dri1 drivers are gone we could
indeed do that.
Or maybe instead a patch to improve the kerneldoc for drm_dev->num_crtc?
-Daniel
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> drivers/gpu/drm/drm_vblank.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 877e2067534f..ad34c235d853 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -575,7 +575,7 @@ EXPORT_SYMBOL(drm_vblank_init);
> */
> bool drm_dev_has_vblank(const struct drm_device *dev)
> {
> - return dev->num_crtcs != 0;
> + return !!dev->vblank;
> }
> EXPORT_SYMBOL(drm_dev_has_vblank);
>
> --
> 2.39.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <robdclark@gmail.com>
Cc: dri-devel@lists.freedesktop.org,
Rob Clark <robdclark@chromium.org>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/vblank: Simplify drm_dev_has_vblank()
Date: Tue, 4 Apr 2023 22:46:00 +0200 [thread overview]
Message-ID: <ZCyMiERvNHTP45o8@phenom.ffwll.local> (raw)
In-Reply-To: <20230403160735.1211468-1-robdclark@gmail.com>
On Mon, Apr 03, 2023 at 09:07:35AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> What does vblank have to do with num_crtcs? Well, this was technically
> correct, but you'd have to go look at where num_crtcs is initialized to
> understand why. Lets just replace it with the simpler and more obvious
> check.
If you want to fix this, then I think the right fix is to rename num_crtcs
to be something like num_vblank_crtcs. It's a historical accident back
when vblanks without kms was a thing.
Plan B is someone gets really busy and fixes up the entire vblank mess and
moves it into drm_crtc struct. Now that the dri1 drivers are gone we could
indeed do that.
Or maybe instead a patch to improve the kerneldoc for drm_dev->num_crtc?
-Daniel
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> drivers/gpu/drm/drm_vblank.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 877e2067534f..ad34c235d853 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -575,7 +575,7 @@ EXPORT_SYMBOL(drm_vblank_init);
> */
> bool drm_dev_has_vblank(const struct drm_device *dev)
> {
> - return dev->num_crtcs != 0;
> + return !!dev->vblank;
> }
> EXPORT_SYMBOL(drm_dev_has_vblank);
>
> --
> 2.39.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2023-04-04 20:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 16:07 [PATCH] drm/vblank: Simplify drm_dev_has_vblank() Rob Clark
2023-04-03 16:07 ` Rob Clark
2023-04-03 16:23 ` Ville Syrjälä
2023-04-03 16:23 ` Ville Syrjälä
2023-04-03 16:28 ` Randy Dunlap
2023-04-03 16:28 ` Randy Dunlap
2023-04-04 20:46 ` Daniel Vetter [this message]
2023-04-04 20:46 ` Daniel Vetter
2023-04-04 21:00 ` Ville Syrjälä
2023-04-05 8:10 ` Daniel Vetter
2023-04-05 8:10 ` Daniel Vetter
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=ZCyMiERvNHTP45o8@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robdclark@chromium.org \
--cc=robdclark@gmail.com \
--cc=tzimmermann@suse.de \
/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.