On ke, 2016-07-27 at 12:14 +0100, Chris Wilson wrote:
> static int i915_gem_object_list_info(struct seq_file *m, void *data)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a24d31e3e014..b6b9a1f78238 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2127,8 +2127,6 @@ struct drm_i915_gem_object_ops {
> */
> #define INTEL_MAX_SPRITE_BITS_PER_PIPE 5
> #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
> -#define INTEL_FRONTBUFFER_BITS \
> - (INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES)
Should we have a BUILD_BUG_ON to make sure we have a fit?
> #define INTEL_FRONTBUFFER_PRIMARY(pipe) \
> (1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> #define INTEL_FRONTBUFFER_CURSOR(pipe) \
> @@ -2216,7 +2214,7 @@ struct drm_i915_gem_object {
> unsigned int cache_level:3;
> unsigned int cache_dirty:1;
>
> - unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
> + atomic_t frontbuffer_bits;
>
> unsigned int has_wc_mmap;
> /** Count of VMA actually bound by this object */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7db0808f6961..bc5bc5ccdde0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4031,7 +4031,7 @@ void i915_gem_free_object(struct drm_gem_object
> *gem_obj)
> if (obj->stolen)
> i915_gem_object_unpin_pages(obj);
>
> - WARN_ON(obj->frontbuffer_bits);
> + WARN_ON(atomic_read(&obj->frontbuffer_bits));
>
> if (obj->pages && obj->madv == I915_MADV_WILLNEED &&
> dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES &&
> @@ -4549,16 +4549,20 @@ void i915_gem_track_fb(struct drm_i915_gem_object
> *old,
> struct drm_i915_gem_object *new,
> unsigned frontbuffer_bits)
> {
> + /* Control of individual bits within the bitfield are guarded by
'bitfield' refers to specific C construct, so not the appropriate term
here now that it is removed. In this commit it is readable, but for
future I think just confusing.
> static void i9xx_update_primary_plane(struct drm_plane *primary,
> @@ -13807,19 +13808,12 @@ static void intel_atomic_track_fbs(struct
> drm_atomic_state *state)
> {
> struct drm_plane_state *old_plane_state;
> struct drm_plane *plane;
> - struct drm_i915_gem_object *obj, *old_obj;
> - struct intel_plane *intel_plane;
> int i;
>
> - mutex_lock(&state->dev->struct_mutex);
> - for_each_plane_in_state(state, plane, old_plane_state, i) {
> - obj = intel_fb_obj(plane->state->fb);
> - old_obj = intel_fb_obj(old_plane_state->fb);
> - intel_plane = to_intel_plane(plane);
> -
> - i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
> - }
> - mutex_unlock(&state->dev->struct_mutex);
> + for_each_plane_in_state(state, plane, old_plane_state, i)
> + i915_gem_track_fb(intel_fb_obj(old_plane_state->fb),
> + intel_fb_obj(plane->state->fb),
> + to_intel_plane(plane)->frontbuffer_bit);
> }
>
These unrelated changes should not be squashed, me thinks. I expect
less intermingling in the future.
Reviewed-by: Joonas Lahtinen <[email protected]>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
