Re: [PATCH 02/26] drm: Switch DRIVER_ flags to an enum

From: Sam Ravnborg <sam@ravnborg.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 02/26] drm: Switch DRIVER_ flags to an enum
Date: Thu, 24 Jan 2019 18:38:33 +0100	[thread overview]
Message-ID: <20190124173833.GA31836@ravnborg.org> (raw)
In-Reply-To: <20190124165831.16427-3-daniel.vetter@ffwll.ch>

Hi Daniel.

> +
> +	/**
> +	 * @DRIVER_HAVE_DMA:
> +	 *
> +	 * Driver supports DMA, the userspace DMA API will be supported. Only
> +	 * for legacy drivers. Do not use.
> +	 */
> +	DRIVER_HAVE_DMA			= BIT(4),

What about grouping all the "legacy, do not use" flags in the bottom.
Maybe counting backwards.

Then one does not have "noise" in-between when trying to rad and understand the
relevant flags for a new driver.
Unless I am mistaken nothing should break because we change the bit for a certain
function, but I see you kept the current order thus the current vlaues.

> @@ -662,7 +766,7 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>   * @feature: feature flag
>   *
>   * This checks @dev for driver features, see &drm_driver.driver_features,
> - * &drm_device.driver_features, and the various DRIVER_\* flags.
> + * &drm_device.driver_features, and the various &enum drm_driver_feature flags.
>   *
>   * Returns true if the @feature is supported, false otherwise.
>   */

Thanks for fixing this - I had a patch floating to do the same.
But this fix is better than what I did.

With or without a change in ordering you can add:

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel