* [PATCH] drm/i915: Handle YUV subpixel support better
@ 2019-03-18 14:07 Maarten Lankhorst
2019-03-18 14:18 ` Ville Syrjälä
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2019-03-18 14:07 UTC (permalink / raw)
To: intel-gfx
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);
+ }
+
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
@ 2019-03-18 14:18 ` Ville Syrjälä
2019-03-18 15:13 ` Maarten Lankhorst
2019-03-18 17:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2019-03-18 14:18 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-18 14:18 ` Ville Syrjälä
@ 2019-03-18 15:13 ` Maarten Lankhorst
2019-03-18 18:15 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Maarten Lankhorst @ 2019-03-18 15:13 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: 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. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Handle YUV subpixel support better
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
2019-03-18 14:18 ` Ville Syrjälä
@ 2019-03-18 17:38 ` Patchwork
2019-03-18 17:39 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-03-18 17:38 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Handle YUV subpixel support better
URL : https://patchwork.freedesktop.org/series/58130/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
383e00c03163 drm/i915: Handle YUV subpixel support better
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:38: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#38: FILE: drivers/gpu/drm/i915/intel_sprite.c:299:
+ hsub = vsub = max(fb->format->hsub, fb->format->vsub);
total: 0 errors, 1 warnings, 1 checks, 44 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: Handle YUV subpixel support better
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
2019-03-18 14:18 ` Ville Syrjälä
2019-03-18 17:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2019-03-18 17:39 ` Patchwork
2019-03-18 17:58 ` ✓ Fi.CI.BAT: success " Patchwork
2019-03-19 1:44 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-03-18 17:39 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Handle YUV subpixel support better
URL : https://patchwork.freedesktop.org/series/58130/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Handle YUV subpixel support better
+drivers/gpu/drm/i915/intel_sprite.c:299:31: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_sprite.c:299:31: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Handle YUV subpixel support better
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
` (2 preceding siblings ...)
2019-03-18 17:39 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-03-18 17:58 ` Patchwork
2019-03-19 1:44 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-03-18 17:58 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Handle YUV subpixel support better
URL : https://patchwork.freedesktop.org/series/58130/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5767 -> Patchwork_12497
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/58130/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12497 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_busy@basic-flip-a:
- fi-gdg-551: PASS -> FAIL [fdo#103182] +1
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
- fi-kbl-7567u: PASS -> SKIP [fdo#109271] +27
#### Possible fixes ####
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: FAIL [fdo#103167] -> PASS
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
#### Warnings ####
* igt@i915_selftest@live_contexts:
- fi-icl-u3: INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] -> DMESG-FAIL [fdo#105079]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-7567u: DMESG-FAIL [fdo#105079] -> SKIP [fdo#109271]
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
Participating hosts (49 -> 41)
------------------------------
Missing (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5767 -> Patchwork_12497
CI_DRM_5767: 289bd1852756ddd2779c32cd13ae10e7bf44faca @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4888: 71ad19eb8fe4f0eecae3bf063e107293b90b9abc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12497: 383e00c03163ab01934fdc4f6a2a1560420e511f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
383e00c03163 drm/i915: Handle YUV subpixel support better
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12497/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-18 15:13 ` Maarten Lankhorst
@ 2019-03-18 18:15 ` Ville Syrjälä
2019-03-19 7:28 ` Maarten Lankhorst
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2019-03-18 18:15 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915: Handle YUV subpixel support better
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
` (3 preceding siblings ...)
2019-03-18 17:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-03-19 1:44 ` Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-03-19 1:44 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Handle YUV subpixel support better
URL : https://patchwork.freedesktop.org/series/58130/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5767_full -> Patchwork_12497_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12497_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12497_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12497_full:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@mock_vma:
- shard-iclb: PASS -> INCOMPLETE
#### Warnings ####
* igt@gem_exec_schedule@in-order-vebox:
- shard-apl: PASS -> ( 2 PASS ) +55
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
- shard-iclb: PASS -> FAIL
Known issues
------------
Here are the changes found in Patchwork_12497_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_sseu@invalid-args:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +67
* igt@gem_eio@unwedge-stress:
- shard-snb: NOTRUN -> FAIL [fdo#109661]
* igt@gem_exec_parse@cmd-crossing-page:
- shard-iclb: NOTRUN -> SKIP [fdo#109289]
* igt@gem_exec_schedule@fifo-bsd2:
- shard-snb: NOTRUN -> SKIP [fdo#109271] +69
* igt@gem_mmap_gtt@big-copy-odd:
- shard-iclb: PASS -> TIMEOUT [fdo#109673]
* igt@gem_pwrite@stolen-snoop:
- shard-iclb: NOTRUN -> SKIP [fdo#109277]
* igt@i915_pm_rpm@gem-mmap-gtt:
- shard-skl: PASS -> INCOMPLETE [fdo#107807]
* igt@i915_pm_rpm@pc8-residency:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#107807]
* igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5
* igt@kms_busy@basic-flip-d:
- shard-iclb: NOTRUN -> SKIP [fdo#109278]
* igt@kms_busy@extended-modeset-hang-newfb-render-b:
- shard-snb: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-modeset-hang-newfb-render-d:
- shard-apl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
* igt@kms_busy@extended-pageflip-hang-newfb-render-c:
- shard-apl: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_chamelium@vga-hpd:
- shard-iclb: NOTRUN -> SKIP [fdo#109284] +1
* igt@kms_content_protection@atomic-dpms:
- shard-apl: NOTRUN -> FAIL [fdo#108739]
* igt@kms_cursor_crc@cursor-128x128-suspend:
- shard-skl: PASS -> INCOMPLETE [fdo#104108]
* igt@kms_cursor_crc@cursor-256x256-random:
- shard-apl: PASS -> FAIL [fdo#103232] +1
* igt@kms_cursor_crc@cursor-64x21-random:
- shard-apl: NOTRUN -> FAIL [fdo#103232]
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
- shard-glk: PASS -> FAIL [fdo#104873]
* igt@kms_cursor_legacy@cursor-vs-flip-legacy:
- shard-iclb: PASS -> FAIL [fdo#103355] +1
* igt@kms_flip@flip-vs-suspend-interruptible:
- shard-skl: PASS -> INCOMPLETE [fdo#109507]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
- shard-apl: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
- shard-skl: NOTRUN -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-blt:
- shard-iclb: NOTRUN -> SKIP [fdo#109280] +2
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
- shard-glk: PASS -> FAIL [fdo#103167] +2
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-iclb: PASS -> FAIL [fdo#103167] +5
* igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-gtt:
- shard-iclb: PASS -> FAIL [fdo#105682] / [fdo#109247]
* igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-pwrite:
- shard-hsw: NOTRUN -> SKIP [fdo#109271] +9
* igt@kms_frontbuffer_tracking@psr-rgb565-draw-render:
- shard-iclb: PASS -> FAIL [fdo#109247] +13
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
- shard-kbl: PASS -> INCOMPLETE [fdo#103665]
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-iclb: PASS -> FAIL [fdo#103375]
* igt@kms_psr@basic:
- shard-apl: NOTRUN -> SKIP [fdo#109271] +56
* igt@kms_psr@cursor_render:
- shard-iclb: PASS -> FAIL [fdo#107383]
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: NOTRUN -> SKIP [fdo#109441]
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: PASS -> SKIP [fdo#109441] +1
* igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-iclb: PASS -> FAIL [fdo#104894]
* igt@kms_vblank@pipe-c-wait-busy:
- shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9
* igt@prime_vgem@fence-wait-bsd1:
- shard-iclb: NOTRUN -> SKIP [fdo#109276] +1
* igt@v3d_get_param@get-bad-param:
- shard-iclb: NOTRUN -> SKIP [fdo#109315]
#### Possible fixes ####
* igt@gem_create@create-clear:
- shard-hsw: INCOMPLETE [fdo#103540] -> PASS
* igt@gem_pwrite@big-cpu-fbr:
- shard-iclb: TIMEOUT [fdo#109673] -> PASS
* igt@kms_color@pipe-a-degamma:
- shard-apl: FAIL [fdo#104782] / [fdo#108145] -> PASS
* igt@kms_cursor_crc@cursor-256x85-sliding:
- shard-apl: FAIL [fdo#103232] -> PASS +3
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-iclb: FAIL [fdo#103375] -> PASS
* igt@kms_flip@2x-flip-vs-expired-vblank:
- shard-glk: FAIL [fdo#105363] -> PASS
* igt@kms_flip@flip-vs-expired-vblank:
- shard-apl: FAIL [fdo#102887] / [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
- shard-apl: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-glk: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
- shard-iclb: FAIL [fdo#109247] -> PASS +13
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: FAIL [fdo#103167] -> PASS +4
* igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
- shard-iclb: FAIL [fdo#105682] / [fdo#109247] -> PASS +1
* {igt@kms_plane@pixel-format-pipe-b-planes}:
- shard-apl: FAIL [fdo#110033] -> PASS +1
* {igt@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
- shard-glk: SKIP [fdo#109271] -> PASS +2
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-snb: INCOMPLETE [fdo#105411] -> PASS
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: FAIL [fdo#108145] -> PASS
* {igt@kms_plane_multiple@atomic-pipe-c-tiling-none}:
- shard-glk: FAIL [fdo#110037] -> PASS +1
* igt@kms_psr@cursor_blt:
- shard-iclb: FAIL [fdo#107383] -> PASS +2
* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: SKIP [fdo#109441] -> PASS +2
* igt@kms_rotation_crc@multiplane-rotation:
- shard-iclb: DMESG-WARN [fdo#106885] -> PASS
* igt@kms_setmode@basic:
- shard-apl: FAIL [fdo#99912] -> PASS
- shard-kbl: FAIL [fdo#99912] -> PASS
#### Warnings ####
* igt@gem_ppgtt@blt-vs-render-ctxn:
- shard-iclb: INCOMPLETE [fdo#107713] / [fdo#109766] / [fdo#109801] -> DMESG-WARN [fdo#109801]
* igt@kms_busy@extended-modeset-hang-newfb-render-b:
- shard-apl: DMESG-WARN [fdo#107956] -> ( 2 DMESG-WARN ) [fdo#107956]
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-e:
- shard-apl: SKIP [fdo#109271] / [fdo#109278] -> ( 2 SKIP ) [fdo#109271] / [fdo#109278] +1
* igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt:
- shard-apl: SKIP [fdo#109271] -> ( 2 SKIP ) [fdo#109271] +20
* igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
- shard-glk: SKIP [fdo#109271] / [fdo#109278] -> FAIL [fdo#110098]
* igt@kms_sysfs_edid_timing:
- shard-apl: FAIL [fdo#100047] -> ( 2 FAIL ) [fdo#100047]
### Piglit changes ###
#### Issues hit ####
* spec@arb_fragment_shader_interlock@arb_fragment_shader_interlock-image-load-store:
- pig-hsw-4770r: NOTRUN -> FAIL [fdo#109980]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
[fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108739]: https://bugs.freedesktop.org/show_bug.cgi?id=108739
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
[fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
[fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
[fdo#109766]: https://bugs.freedesktop.org/show_bug.cgi?id=109766
[fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
[fdo#109980]: https://bugs.freedesktop.org/show_bug.cgi?id=109980
[fdo#110033]: https://bugs.freedesktop.org/show_bug.cgi?id=110033
[fdo#110037]: https://bugs.freedesktop.org/show_bug.cgi?id=110037
[fdo#110098]: https://bugs.freedesktop.org/show_bug.cgi?id=110098
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (9 -> 10)
------------------------------
Additional (1): pig-hsw-4770r
Build changes
-------------
* Linux: CI_DRM_5767 -> Patchwork_12497
CI_DRM_5767: 289bd1852756ddd2779c32cd13ae10e7bf44faca @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4888: 71ad19eb8fe4f0eecae3bf063e107293b90b9abc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12497: 383e00c03163ab01934fdc4f6a2a1560420e511f @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12497/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-18 18:15 ` Ville Syrjälä
@ 2019-03-19 7:28 ` Maarten Lankhorst
2019-03-19 11:15 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Maarten Lankhorst @ 2019-03-19 7:28 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-19 7:28 ` Maarten Lankhorst
@ 2019-03-19 11:15 ` Ville Syrjälä
2019-03-19 12:45 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2019-03-19 11:15 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Handle YUV subpixel support better
2019-03-19 11:15 ` Ville Syrjälä
@ 2019-03-19 12:45 ` Ville Syrjälä
0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-03-19 12:45 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-03-19 12:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-18 14:07 [PATCH] drm/i915: Handle YUV subpixel support better Maarten Lankhorst
2019-03-18 14:18 ` Ville Syrjälä
2019-03-18 15:13 ` Maarten Lankhorst
2019-03-18 18:15 ` Ville Syrjälä
2019-03-19 7:28 ` Maarten Lankhorst
2019-03-19 11:15 ` Ville Syrjälä
2019-03-19 12:45 ` Ville Syrjälä
2019-03-18 17:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-03-18 17:39 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-03-18 17:58 ` ✓ Fi.CI.BAT: success " Patchwork
2019-03-19 1:44 ` ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.