| Message ID | 20190318140718.1619-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Handle YUV subpixel support better | expand |
On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 268fb34ff0e2..862fc172042f 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > { > const struct drm_framebuffer *fb = plane_state->base.fb; > struct drm_rect *src = &plane_state->base.src; > - u32 src_x, src_y, src_w, src_h; > + u32 src_x, src_y, src_w, src_h, hsub, vsub; > + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); > > /* > * Hardware doesn't handle subpixel coordinates. > @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > src->y1 = src_y << 16; > src->y2 = (src_y + src_h) << 16; > > - if (fb->format->is_yuv && > - (src_x & 1 || src_w & 1)) { > - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", > - src_x, src_w); > + if (!fb->format->is_yuv) > + return 0; > + > + /* YUV specific checks */ > + if (!rotated) { > + hsub = fb->format->hsub; > + vsub = fb->format->vsub; > + } else { > + hsub = vsub = max(fb->format->hsub, fb->format->vsub); Why this? From the looks of things there should be no need to deal with rotation in this function at all. > + } > + > + if (src_x % hsub || src_w % hsub) { > + DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of %u for %sYUV planes\n", > + src_x, src_w, hsub, rotated ? "rotated " : ""); > return -EINVAL; > } > > - if (fb->format->is_yuv && > - fb->format->num_planes > 1 && > - (src_y & 1 || src_h & 1)) { > - DRM_DEBUG_KMS("src y/h (%u, %u) must be a multiple of 2 for planar YUV planes\n", > - src_y, src_h); > + if (src_y % vsub || src_h % vsub) { > + DRM_DEBUG_KMS("src y/h (%u, %u) must be a multiple of %u for %sYUV planes\n", > + src_y, src_h, vsub, rotated ? "rotated " : ""); > return -EINVAL; > } > > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 18-03-2019 om 15:18 schreef Ville Syrjälä: > On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- >> 1 file changed, 19 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c >> index 268fb34ff0e2..862fc172042f 100644 >> --- a/drivers/gpu/drm/i915/intel_sprite.c >> +++ b/drivers/gpu/drm/i915/intel_sprite.c >> @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) >> { >> const struct drm_framebuffer *fb = plane_state->base.fb; >> struct drm_rect *src = &plane_state->base.src; >> - u32 src_x, src_y, src_w, src_h; >> + u32 src_x, src_y, src_w, src_h, hsub, vsub; >> + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); >> >> /* >> * Hardware doesn't handle subpixel coordinates. >> @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) >> src->y1 = src_y << 16; >> src->y2 = (src_y + src_h) << 16; >> >> - if (fb->format->is_yuv && >> - (src_x & 1 || src_w & 1)) { >> - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", >> - src_x, src_w); >> + if (!fb->format->is_yuv) >> + return 0; >> + >> + /* YUV specific checks */ >> + if (!rotated) { >> + hsub = fb->format->hsub; >> + vsub = fb->format->vsub; >> + } else { >> + hsub = vsub = max(fb->format->hsub, fb->format->vsub); > Why this? From the looks of things there should be no need to deal with > rotation in this function at all. I wrote a dumb test that fails if I rotate YUYV. https://patchwork.freedesktop.org/patch/286170/ Corrupted image: (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_rotation(90°) (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_position(18,33) (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_size(44x65) (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_position(64,64) (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_size (256x256) I had a 80x128 fb, only showing the center part which should be white, with a black border around it to cause CRC errors if we mess up clipping. The scaling works fine, but the clipping does not in this case. I am getting a corrupted plane on screen which is mostly white, but with black dots in each tile. Scaling just magnifies this corruption. :)
On Mon, Mar 18, 2019 at 04:13:57PM +0100, Maarten Lankhorst wrote: > Op 18-03-2019 om 15:18 schreef Ville Syrjälä: > > On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- > >> 1 file changed, 19 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > >> index 268fb34ff0e2..862fc172042f 100644 > >> --- a/drivers/gpu/drm/i915/intel_sprite.c > >> +++ b/drivers/gpu/drm/i915/intel_sprite.c > >> @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > >> { > >> const struct drm_framebuffer *fb = plane_state->base.fb; > >> struct drm_rect *src = &plane_state->base.src; > >> - u32 src_x, src_y, src_w, src_h; > >> + u32 src_x, src_y, src_w, src_h, hsub, vsub; > >> + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); > >> > >> /* > >> * Hardware doesn't handle subpixel coordinates. > >> @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > >> src->y1 = src_y << 16; > >> src->y2 = (src_y + src_h) << 16; > >> > >> - if (fb->format->is_yuv && > >> - (src_x & 1 || src_w & 1)) { > >> - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", > >> - src_x, src_w); > >> + if (!fb->format->is_yuv) > >> + return 0; > >> + > >> + /* YUV specific checks */ > >> + if (!rotated) { > >> + hsub = fb->format->hsub; > >> + vsub = fb->format->vsub; > >> + } else { > >> + hsub = vsub = max(fb->format->hsub, fb->format->vsub); > > Why this? From the looks of things there should be no need to deal with > > rotation in this function at all. > > I wrote a dumb test that fails if I rotate YUYV. > > https://patchwork.freedesktop.org/patch/286170/ > > Corrupted image: > > (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_rotation(90°) > (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_position(18,33) > (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_size(44x65) > (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_position(64,64) > (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_size (256x256) > > I had a 80x128 fb, only showing the center part which should be white, with a black border around it to cause CRC errors if we mess up clipping. > > The scaling works fine, but the clipping does not in this case. I am getting a corrupted plane on screen which is mostly white, but with black dots in each tile. > > Scaling just magnifies this corruption. :) Hmm. I just poked my KBL a bit and it is also showing curious behaviour. Even with 90/270 rotation it is in fact the TILEOFF X coordinate that needs to be even (actually the hw just appears to ignore the lsb). I can make the Y coordinate odd, and the image still looks correct to my eyes. So feels like someone forgot to to remove a (x&~1) from the hw when they added the 90/270 rotation, and yet they went to the trouble of making odd Y coordinates work correctly. Quite stange. Width/height being odd seems to handled just fine by the hw.
Op 18-03-2019 om 19:15 schreef Ville Syrjälä: > On Mon, Mar 18, 2019 at 04:13:57PM +0100, Maarten Lankhorst wrote: >> Op 18-03-2019 om 15:18 schreef Ville Syrjälä: >>> On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>>> --- >>>> drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- >>>> 1 file changed, 19 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c >>>> index 268fb34ff0e2..862fc172042f 100644 >>>> --- a/drivers/gpu/drm/i915/intel_sprite.c >>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c >>>> @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) >>>> { >>>> const struct drm_framebuffer *fb = plane_state->base.fb; >>>> struct drm_rect *src = &plane_state->base.src; >>>> - u32 src_x, src_y, src_w, src_h; >>>> + u32 src_x, src_y, src_w, src_h, hsub, vsub; >>>> + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); >>>> >>>> /* >>>> * Hardware doesn't handle subpixel coordinates. >>>> @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) >>>> src->y1 = src_y << 16; >>>> src->y2 = (src_y + src_h) << 16; >>>> >>>> - if (fb->format->is_yuv && >>>> - (src_x & 1 || src_w & 1)) { >>>> - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", >>>> - src_x, src_w); >>>> + if (!fb->format->is_yuv) >>>> + return 0; >>>> + >>>> + /* YUV specific checks */ >>>> + if (!rotated) { >>>> + hsub = fb->format->hsub; >>>> + vsub = fb->format->vsub; >>>> + } else { >>>> + hsub = vsub = max(fb->format->hsub, fb->format->vsub); >>> Why this? From the looks of things there should be no need to deal with >>> rotation in this function at all. >> I wrote a dumb test that fails if I rotate YUYV. >> >> https://patchwork.freedesktop.org/patch/286170/ >> >> Corrupted image: >> >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_rotation(90°) >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_position(18,33) >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_size(44x65) >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_position(64,64) >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_size (256x256) >> >> I had a 80x128 fb, only showing the center part which should be white, with a black border around it to cause CRC errors if we mess up clipping. >> >> The scaling works fine, but the clipping does not in this case. I am getting a corrupted plane on screen which is mostly white, but with black dots in each tile. >> >> Scaling just magnifies this corruption. :) > Hmm. I just poked my KBL a bit and it is also showing curious > behaviour. Even with 90/270 rotation it is in fact the TILEOFF > X coordinate that needs to be even (actually the hw just appears > to ignore the lsb). I can make the Y coordinate odd, and the image > still looks correct to my eyes. So feels like someone forgot to > to remove a (x&~1) from the hw when they added the 90/270 rotation, > and yet they went to the trouble of making odd Y coordinates work > correctly. Quite stange. > > Width/height being odd seems to handled just fine by the hw. > Hmm does that mean we should keep the original checks in place while checking format->h/vsub, and on top reject the unrotated Y coordinate being a multiple of hsub when rotating? ~Maarten
On Tue, Mar 19, 2019 at 08:28:58AM +0100, Maarten Lankhorst wrote: > Op 18-03-2019 om 19:15 schreef Ville Syrjälä: > > On Mon, Mar 18, 2019 at 04:13:57PM +0100, Maarten Lankhorst wrote: > >> Op 18-03-2019 om 15:18 schreef Ville Syrjälä: > >>> On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: > >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >>>> --- > >>>> drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- > >>>> 1 file changed, 19 insertions(+), 10 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > >>>> index 268fb34ff0e2..862fc172042f 100644 > >>>> --- a/drivers/gpu/drm/i915/intel_sprite.c > >>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c > >>>> @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > >>>> { > >>>> const struct drm_framebuffer *fb = plane_state->base.fb; > >>>> struct drm_rect *src = &plane_state->base.src; > >>>> - u32 src_x, src_y, src_w, src_h; > >>>> + u32 src_x, src_y, src_w, src_h, hsub, vsub; > >>>> + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); > >>>> > >>>> /* > >>>> * Hardware doesn't handle subpixel coordinates. > >>>> @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > >>>> src->y1 = src_y << 16; > >>>> src->y2 = (src_y + src_h) << 16; > >>>> > >>>> - if (fb->format->is_yuv && > >>>> - (src_x & 1 || src_w & 1)) { > >>>> - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", > >>>> - src_x, src_w); > >>>> + if (!fb->format->is_yuv) > >>>> + return 0; > >>>> + > >>>> + /* YUV specific checks */ > >>>> + if (!rotated) { > >>>> + hsub = fb->format->hsub; > >>>> + vsub = fb->format->vsub; > >>>> + } else { > >>>> + hsub = vsub = max(fb->format->hsub, fb->format->vsub); > >>> Why this? From the looks of things there should be no need to deal with > >>> rotation in this function at all. > >> I wrote a dumb test that fails if I rotate YUYV. > >> > >> https://patchwork.freedesktop.org/patch/286170/ > >> > >> Corrupted image: > >> > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_rotation(90°) > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_position(18,33) > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_size(44x65) > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_position(64,64) > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_size (256x256) > >> > >> I had a 80x128 fb, only showing the center part which should be white, with a black border around it to cause CRC errors if we mess up clipping. > >> > >> The scaling works fine, but the clipping does not in this case. I am getting a corrupted plane on screen which is mostly white, but with black dots in each tile. > >> > >> Scaling just magnifies this corruption. :) > > Hmm. I just poked my KBL a bit and it is also showing curious > > behaviour. Even with 90/270 rotation it is in fact the TILEOFF > > X coordinate that needs to be even (actually the hw just appears > > to ignore the lsb). I can make the Y coordinate odd, and the image > > still looks correct to my eyes. So feels like someone forgot to > > to remove a (x&~1) from the hw when they added the 90/270 rotation, > > and yet they went to the trouble of making odd Y coordinates work > > correctly. Quite stange. > > > > Width/height being odd seems to handled just fine by the hw. > > > Hmm does that mean we should keep the original checks in place while checking format->h/vsub, and on top reject the unrotated Y coordinate being a multiple of hsub when rotating? Not quite sure. Based on what I see we could actually just swap the coordinates (or do the check after the coordinates are already rotated) and it should still work. But I didn't check if that would still work when the scaler is involved.
On Tue, Mar 19, 2019 at 01:15:55PM +0200, Ville Syrjälä wrote: > On Tue, Mar 19, 2019 at 08:28:58AM +0100, Maarten Lankhorst wrote: > > Op 18-03-2019 om 19:15 schreef Ville Syrjälä: > > > On Mon, Mar 18, 2019 at 04:13:57PM +0100, Maarten Lankhorst wrote: > > >> Op 18-03-2019 om 15:18 schreef Ville Syrjälä: > > >>> On Mon, Mar 18, 2019 at 03:07:18PM +0100, Maarten Lankhorst wrote: > > >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > >>>> --- > > >>>> drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- > > >>>> 1 file changed, 19 insertions(+), 10 deletions(-) > > >>>> > > >>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > >>>> index 268fb34ff0e2..862fc172042f 100644 > > >>>> --- a/drivers/gpu/drm/i915/intel_sprite.c > > >>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c > > >>>> @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > > >>>> { > > >>>> const struct drm_framebuffer *fb = plane_state->base.fb; > > >>>> struct drm_rect *src = &plane_state->base.src; > > >>>> - u32 src_x, src_y, src_w, src_h; > > >>>> + u32 src_x, src_y, src_w, src_h, hsub, vsub; > > >>>> + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); > > >>>> > > >>>> /* > > >>>> * Hardware doesn't handle subpixel coordinates. > > >>>> @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) > > >>>> src->y1 = src_y << 16; > > >>>> src->y2 = (src_y + src_h) << 16; > > >>>> > > >>>> - if (fb->format->is_yuv && > > >>>> - (src_x & 1 || src_w & 1)) { > > >>>> - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", > > >>>> - src_x, src_w); > > >>>> + if (!fb->format->is_yuv) > > >>>> + return 0; > > >>>> + > > >>>> + /* YUV specific checks */ > > >>>> + if (!rotated) { > > >>>> + hsub = fb->format->hsub; > > >>>> + vsub = fb->format->vsub; > > >>>> + } else { > > >>>> + hsub = vsub = max(fb->format->hsub, fb->format->vsub); > > >>> Why this? From the looks of things there should be no need to deal with > > >>> rotation in this function at all. > > >> I wrote a dumb test that fails if I rotate YUYV. > > >> > > >> https://patchwork.freedesktop.org/patch/286170/ > > >> > > >> Corrupted image: > > >> > > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_rotation(90°) > > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_position(18,33) > > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: src_set_size(44x65) > > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_position(64,64) > > >> (kms_yuv:1155) igt_kms-DEBUG: display: A.0: plane_set_size (256x256) > > >> > > >> I had a 80x128 fb, only showing the center part which should be white, with a black border around it to cause CRC errors if we mess up clipping. > > >> > > >> The scaling works fine, but the clipping does not in this case. I am getting a corrupted plane on screen which is mostly white, but with black dots in each tile. > > >> > > >> Scaling just magnifies this corruption. :) > > > Hmm. I just poked my KBL a bit and it is also showing curious > > > behaviour. Even with 90/270 rotation it is in fact the TILEOFF > > > X coordinate that needs to be even (actually the hw just appears > > > to ignore the lsb). I can make the Y coordinate odd, and the image > > > still looks correct to my eyes. So feels like someone forgot to > > > to remove a (x&~1) from the hw when they added the 90/270 rotation, > > > and yet they went to the trouble of making odd Y coordinates work > > > correctly. Quite stange. > > > > > > Width/height being odd seems to handled just fine by the hw. > > > > > Hmm does that mean we should keep the original checks in place while checking format->h/vsub, and on top reject the unrotated Y coordinate being a multiple of hsub when rotating? > > Not quite sure. Based on what I see we could actually just swap the > coordinates (or do the check after the coordinates are already rotated) > and it should still work. But I didn't check if that would still work > when the scaler is involved. Hmm. The spec disagrees with this observed behaviour of PLANE_OFFSET. It claims our current code should be fine. PLANE_SIZE also has this slightly confusing table for GLK+: PixelFormat Rotate Width Height YUV 420 Planar - NV12 All Even Even YUV 420 Planar - P01x All Even Even YUV 422 All Even Even RGB565 90, 270 Even Even which pretty much matches the w/h part of your patch.
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 268fb34ff0e2..862fc172042f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -269,7 +269,8 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) { const struct drm_framebuffer *fb = plane_state->base.fb; struct drm_rect *src = &plane_state->base.src; - u32 src_x, src_y, src_w, src_h; + u32 src_x, src_y, src_w, src_h, hsub, vsub; + bool rotated = drm_rotation_90_or_270(plane_state->base.rotation); /* * Hardware doesn't handle subpixel coordinates. @@ -287,18 +288,26 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state) src->y1 = src_y << 16; src->y2 = (src_y + src_h) << 16; - if (fb->format->is_yuv && - (src_x & 1 || src_w & 1)) { - DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n", - src_x, src_w); + if (!fb->format->is_yuv) + return 0; + + /* YUV specific checks */ + if (!rotated) { + hsub = fb->format->hsub; + vsub = fb->format->vsub; + } else { + hsub = vsub = max(fb->format->hsub, fb->format->vsub); + } + + if (src_x % hsub || src_w % hsub) { + DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of %u for %sYUV planes\n", + src_x, src_w, hsub, rotated ? "rotated " : ""); return -EINVAL; } - if (fb->format->is_yuv && - fb->format->num_planes > 1 && - (src_y & 1 || src_h & 1)) { - DRM_DEBUG_KMS("src y/h (%u, %u) must be a multiple of 2 for planar YUV planes\n", - src_y, src_h); + if (src_y % vsub || src_h % vsub) { + DRM_DEBUG_KMS("src y/h (%u, %u) must be a multiple of %u for %sYUV planes\n", + src_y, src_h, vsub, rotated ? "rotated " : ""); return -EINVAL; }
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)