I'd fix the below code style corrections, the functionality is ok.
On ti, 2015-03-17 at 15:45 +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> For now only default implementation defaulting to normal view.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 862aa46..fe11e99 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2249,6 +2249,16 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height,
> fb_format_modifier));
> }
>
> +static
> +int intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
Rather make this as it's elsewhere (static and return type on the same
line):
+static int
+intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
> + struct drm_framebuffer *fb,
> + const struct drm_plane_state *plane_state)
> +{
> + *view = i915_ggtt_view_normal;
> +
> + return 0;
> +}
> +
> int
> intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> struct drm_framebuffer *fb,
> @@ -2258,6 +2268,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> struct drm_device *dev = fb->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct i915_ggtt_view view;
> u32 alignment;
> int ret;
>
> @@ -2294,6 +2305,10 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
> + if (ret < 0)
Why not just "if (ret)" for consistency?
> + return ret;
> +
> /* Note that the w/a also requires 64 PTE of padding following the
> * bo. We currently fill all unused PTE with the shadow page and so
> * we should always have valid PTE following the scanout preventing
> @@ -2313,7 +2328,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>
> dev_priv->mm.interruptible = false;
> ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined,
> - &i915_ggtt_view_normal);
> + &view);
> if (ret)
> goto err_interruptible;
>
> @@ -2333,7 +2348,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> return 0;
>
> err_unpin:
> - i915_gem_object_unpin_from_display_plane(obj, &i915_ggtt_view_normal);
> + i915_gem_object_unpin_from_display_plane(obj, &view);
> err_interruptible:
> dev_priv->mm.interruptible = true;
> intel_runtime_pm_put(dev_priv);
> @@ -2344,11 +2359,16 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
> const struct drm_plane_state *plane_state)
> {
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct i915_ggtt_view view;
> + int ret;
>
> WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
>
> + ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
> + WARN_ONCE(ret < 0, "Couldn't get view from plane state!");
Again, just WARN_ONCE(ret, ...) as the function doesn't have need to
return any positive integer result.
> +
> i915_gem_object_unpin_fence(obj);
> - i915_gem_object_unpin_from_display_plane(obj, &i915_ggtt_view_normal);
> + i915_gem_object_unpin_from_display_plane(obj, &view);
> }
>
> /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
