Hi,

On 04/02/2015 05:54 AM, Jindal, Sonika wrote:
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view
*view, struct drm_framebuffer *fb,
          return -EINVAL;
      }

+    switch (fb->pixel_format) {
+    case DRM_FORMAT_XRGB8888:
+    case DRM_FORMAT_ARGB8888:
+    case DRM_FORMAT_XBGR8888:
+    case DRM_FORMAT_ABGR8888:
+    case DRM_FORMAT_XRGB2101010:
+    case DRM_FORMAT_ARGB2101010:
+    case DRM_FORMAT_XBGR2101010:
+    case DRM_FORMAT_ABGR2101010:
+    case DRM_FORMAT_YUYV:
+    case DRM_FORMAT_YVYU:
+    case DRM_FORMAT_UYVY:
+    case DRM_FORMAT_VYUY:
+    case DRM_FORMAT_NV12:
+        break;
+
+    default:
+        DRM_DEBUG_KMS("Unsupported pixel format:%d for 90/270
rotation!\n",
+              fb->pixel_format);
+        return -EINVAL;
+    }

Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

     drm_plane_check_pixel_format(plane_state->plane, fb->pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.

I am not sure how it will help. drm_plane_check_pixel_format should be
used to check the pixel format of the fb which we should be doing in
some -check functions (I don't think we do that right now?) against what
is supported by the plane.
But to check for the formats which are allowed for 90/270, we would need
this kind of explicit check.

I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.

Yes, I agree, but this is on top of Tvrtko's patch for secondary buffer
mapping where based upon tiling and pixel format we are allowing the
rotated gtt.

Tvrtko,
Can these be moved to the intel_plane_atomic_check()

Good point, I think it can and should. I suppose it was just an oversight during endless rebasing, that I put the Y tiling check in there. So you can move that part as well while you are doing it.

Also highlights the fact we have no negative testing in kms_rotation_crc for this. I mean, trying to rotate by 90/270 linear or X tiled, or wrong pixel format.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to