Re: [PATCH 01/13] drm/i915: Rename agp_type to cache_level

Keith Packard
2011-04-14T12:39:23Z

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

On Thu, 14 Apr 2011 10:03:35 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> ... to clarify just how we use it inside the driver. We still need to
> translate through agp_type for interface into the fake AGP driver.

agp_type has some really confusing semantics...

> - obj->agp_type = AGP_USER_MEMORY;
> + obj->cache_level = I915_CACHE_NONE;

AGP_USER_MEMORY is indeed uncached, so this is correct.

> +/* XXX kill agp_type! */
> +static uint32_t cache_level_to_agp_type(struct drm_device *dev,
> + enum i915_cache_level cache_level)

This should be unsigned int to match the users of this value

> + switch (cache_level) {
> + case I915_CACHE_LLC_MLC:
> + if (INTEL_INFO(dev)->gen >= 6)
> + return AGP_USER_CACHED_MEMORY_LLC_MLC;

I like to see a comment here:

+ /* Fall through ... */

> + case I915_CACHE_LLC:
> + return AGP_USER_CACHED_MEMORY;
> + default:
> + case I915_CACHE_NONE:
> + return AGP_USER_MEMORY;
> + }

> list_for_each_entry(obj, &dev_priv->mm.gtt_list, gtt_list) {
> + int agp_type = cache_level_to_agp_type(dev, obj->cache_level);
> +

This should be 'unsigned int' to match the signature of
intel_gtt_insert_sg_entries and intel_gtt_insert_pages.

--
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org