Re: [PATCH] drm/radeon: Update radeon_get_vblank_counter_kms()

From: Thierry Reding <thierry.reding@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon: Update radeon_get_vblank_counter_kms()
Date: Mon, 4 Jan 2016 12:28:43 +0100	[thread overview]
Message-ID: <20160104112843.GA29079@ulmo> (raw)
In-Reply-To: <567257C6.908@daenzer.net>

[-- Attachment #1.1: Type: text/plain, Size: 862 bytes --]

On Thu, Dec 17, 2015 at 03:35:50PM +0900, Michel Dänzer wrote:
> On 16.12.2015 23:31, Thierry Reding wrote:
> > 
> > -	if (crtc < 0 || crtc >= rdev->num_crtc) {
> > -		DRM_ERROR("Invalid crtc %d\n", crtc);
> > +	if (pipe < 0 || pipe >= rdev->num_crtc) {
> > +		DRM_ERROR("Invalid crtc %u\n", pipe);
> 
> Does it still make sense to test for < 0? Doesn't that generate a
> compiler warning?

I didn't see a compiler warning in my build tests, but you're right,
it's not necessary to check for that because it can't be true. I'll
respin with that fixed up.

> Also, I don't really get the point of the crtc => pipe rename here...

This is primarily for consistency. That way we can reserve "pipe" for
the unsigned integers that denote the DRM index of the CRTC, and keep
"crtc" for cases where we deal with struct drm_crtc objects.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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