diff mbox series

[1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites

Message ID 20191008161441.12721-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites | expand

Commit Message

Ville Syrjälä Oct. 8, 2019, 4:14 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

SNB-BDW support 10:10:10 formats on the sprite planes. Let's expose
them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Ville Syrjälä Oct. 14, 2019, 7:23 p.m. UTC | #1 
On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> URL   : https://patchwork.freedesktop.org/series/67741/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_14725_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_14725_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@gem_eio@in-flight-1us:
>     - shard-snb:          [PASS][1] -> [FAIL][2]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> 
>   * igt@kms_plane@pixel-format-pipe-a-planes:
>     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html

IGT-Version: 1.24-ge501741f
...
Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
(kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)

DRM_FORMAT_ARGB2101010 =  0x30335241
IGT_FORMAT_FLOAT = 0x78464749

{ .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
  .pixman_id = PIXMAN_a2r10g10b10,

{ .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
  .pixman_id = PIXMAN_rgba_float,

if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
    (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
	cnvert_pixman(cvt);
	return;
...
igt_assert_f(false, "Conversion not implemented ...);

So wtf?

Are we somehow compiling igt with an old pixman causing
 #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
 #define PIXMAN_rgba_float PIXMAN_invalid
 #endif
to happen?

But the reference run shows it testing all the fancy YUV formats so
I don't think that can be the case.

And my CHV can run this test with ARGB2101010 just fine so
the code can't be totally broken.
Chris Wilson Oct. 14, 2019, 8:23 p.m. UTC | #2 
Quoting Ville Syrjälä (2019-10-14 20:23:42)
> On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> > URL   : https://patchwork.freedesktop.org/series/67741/
> > State : failure
> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > ====================================================
> > 
> > Summary
> > -------
> > 
> >   **FAILURE**
> > 
> >   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_14725_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_14725_full:
> > 
> > ### IGT changes ###
> > 
> > #### Possible regressions ####
> > 
> >   * igt@gem_eio@in-flight-1us:
> >     - shard-snb:          [PASS][1] -> [FAIL][2]
> >    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
> >    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> > 
> >   * igt@kms_plane@pixel-format-pipe-a-planes:
> >     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
> >    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
> >    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html
> 
> IGT-Version: 1.24-ge501741f
> ...
> Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
> (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)
> 
> DRM_FORMAT_ARGB2101010 =  0x30335241
> IGT_FORMAT_FLOAT = 0x78464749
> 
> { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
>   .pixman_id = PIXMAN_a2r10g10b10,
> 
> { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
>   .pixman_id = PIXMAN_rgba_float,
> 
> if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
>     (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
>         cnvert_pixman(cvt);
>         return;
> ...
> igt_assert_f(false, "Conversion not implemented ...);
> 
> So wtf?
> 
> Are we somehow compiling igt with an old pixman causing
>  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
>  #define PIXMAN_rgba_float PIXMAN_invalid
>  #endif
> to happen?

That's the only way I can see, perhaps stick a #warn in there.
-Chris
Arkadiusz Hiler Oct. 15, 2019, 6:41 a.m. UTC | #3 
On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> > URL   : https://patchwork.freedesktop.org/series/67741/
> > State : failure
> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > ====================================================
> > 
> > Summary
> > -------
> > 
> >   **FAILURE**
> > 
> >   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_14725_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_14725_full:
> > 
> > ### IGT changes ###
> > 
> > #### Possible regressions ####
> > 
> >   * igt@gem_eio@in-flight-1us:
> >     - shard-snb:          [PASS][1] -> [FAIL][2]
> >    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
> >    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> > 
> >   * igt@kms_plane@pixel-format-pipe-a-planes:
> >     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
> >    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
> >    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html
> 
> IGT-Version: 1.24-ge501741f
> ...
> Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
> (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)
> 
> DRM_FORMAT_ARGB2101010 =  0x30335241
> IGT_FORMAT_FLOAT = 0x78464749
> 
> { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
>   .pixman_id = PIXMAN_a2r10g10b10,
> 
> { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
>   .pixman_id = PIXMAN_rgba_float,
> 
> if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
>     (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> 	cnvert_pixman(cvt);
> 	return;
> ...
> igt_assert_f(false, "Conversion not implemented ...);
> 
> So wtf?
> 
> Are we somehow compiling igt with an old pixman causing
>  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
>  #define PIXMAN_rgba_float PIXMAN_invalid
>  #endif
> to happen?

oof, seems like the building machine got downgraded somehow

ci-worker1:~$ dpkg -l '*pixman*'
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                                            Version                      Architecture                 Description
+++-===============================================-============================-============================-===================================================================================================
ii  libpixman-1-0:amd64                             0.34.0-2                     amd64                        pixel-manipulation library for X and cairo
ii  libpixman-1-dev:amd64                           0.34.0-2                     amd64                        pixel-manipulation library for X and cairo (development files)

that's bad...

> But the reference run shows it testing all the fancy YUV formats so
> I don't think that can be the case.

That's the weird bit...

Anyway the building machine needs updating and apt-mark hold.
This can cause fallout and we need to file bugs to limit the noise.

There is quite some queue right now, but hopefully by tomorrow it will
be drained. I'll do the necessary updates and force IGT run to see what
is going to happen in the morning. Then I'll rerun this series.
Petri Latvala Oct. 15, 2019, 9:25 a.m. UTC | #4 
On Tue, Oct 15, 2019 at 09:41:20AM +0300, Arkadiusz Hiler wrote:
> On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> > > == Series Details ==
> > > 
> > > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> > > URL   : https://patchwork.freedesktop.org/series/67741/
> > > State : failure
> > > 
> > > == Summary ==
> > > 
> > > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > > ====================================================
> > > 
> > > Summary
> > > -------
> > > 
> > >   **FAILURE**
> > > 
> > >   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
> > >   verified manually.
> > >   
> > >   If you think the reported changes have nothing to do with the changes
> > >   introduced in Patchwork_14725_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_14725_full:
> > > 
> > > ### IGT changes ###
> > > 
> > > #### Possible regressions ####
> > > 
> > >   * igt@gem_eio@in-flight-1us:
> > >     - shard-snb:          [PASS][1] -> [FAIL][2]
> > >    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
> > >    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> > > 
> > >   * igt@kms_plane@pixel-format-pipe-a-planes:
> > >     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
> > >    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
> > >    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html
> > 
> > IGT-Version: 1.24-ge501741f
> > ...
> > Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
> > (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)
> > 
> > DRM_FORMAT_ARGB2101010 =  0x30335241
> > IGT_FORMAT_FLOAT = 0x78464749
> > 
> > { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
> >   .pixman_id = PIXMAN_a2r10g10b10,
> > 
> > { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
> >   .pixman_id = PIXMAN_rgba_float,
> > 
> > if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> >     (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> > 	cnvert_pixman(cvt);
> > 	return;
> > ...
> > igt_assert_f(false, "Conversion not implemented ...);
> > 
> > So wtf?
> > 
> > Are we somehow compiling igt with an old pixman causing
> >  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
> >  #define PIXMAN_rgba_float PIXMAN_invalid
> >  #endif
> > to happen?
> 
> oof, seems like the building machine got downgraded somehow
> 
> ci-worker1:~$ dpkg -l '*pixman*'
> Desired=Unknown/Install/Remove/Purge/Hold
> | Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
> |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
> ||/ Name                                            Version                      Architecture                 Description
> +++-===============================================-============================-============================-===================================================================================================
> ii  libpixman-1-0:amd64                             0.34.0-2                     amd64                        pixel-manipulation library for X and cairo
> ii  libpixman-1-dev:amd64                           0.34.0-2                     amd64                        pixel-manipulation library for X and cairo (development files)
> 
> that's bad...
> 
> > But the reference run shows it testing all the fancy YUV formats so
> > I don't think that can be the case.
> 
> That's the weird bit...
> 
> Anyway the building machine needs updating and apt-mark hold.
> This can cause fallout and we need to file bugs to limit the noise.
> 
> There is quite some queue right now, but hopefully by tomorrow it will
> be drained. I'll do the necessary updates and force IGT run to see what
> is going to happen in the morning. Then I'll rerun this series.
> 


I don't think the builder ever had a higher version.

The fancy YUV formats work because the runtime lib is new enough, and
the build-time checks for those are as such:

#if CAIRO_VERSION < CAIRO_VERSION_ENCODE(1, 17, 2)
/*
 * We need cairo 1.17.2 to use HDR formats, but the only thing added is a value
 * to cairo_format_t.
 *
 * To prevent going outside the enum, make cairo_format_t an int and define
 * ourselves.
*/

#define	CAIRO_FORMAT_RGB96F (6)
#define	CAIRO_FORMAT_RGBA128F (7)
#define	cairo_format_t int
#endif


  and


igt_skip_on_f(status == CAIRO_STATUS_INVALID_FORMAT &&
	      cairo_version() < CAIRO_VERSION_ENCODE(1, 17, 2),
	      "Cairo version too old, need 1.17.2, have %s\n",
	      cairo_version_string());

igt_skip_on_f(status == CAIRO_STATUS_NO_MEMORY &&
	      pixman_version() < PIXMAN_VERSION_ENCODE(0, 36, 0),
	      "Pixman version too old, need 0.36.0, have %s\n",
	      pixman_version_string());




In other words, the backwards compatibility for the fancy YUV formats
at build-time was easy to sneak in by defining some values, is that
possible to do with the 10bpc stuff? PIXMAN_rgba_float seems to be
just PIXMAN_FORMAT_BYTE(128, PIXMAN_TYPE_RGBA_FLOAT,32,32,32,32),
where PIXMAN_TYPE_RGBA_FLOAT is 11.
Ville Syrjälä Oct. 15, 2019, 11:51 a.m. UTC | #5 
On Tue, Oct 15, 2019 at 12:25:59PM +0300, Petri Latvala wrote:
> On Tue, Oct 15, 2019 at 09:41:20AM +0300, Arkadiusz Hiler wrote:
> > On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> > > > == Series Details ==
> > > > 
> > > > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> > > > URL   : https://patchwork.freedesktop.org/series/67741/
> > > > State : failure
> > > > 
> > > > == Summary ==
> > > > 
> > > > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > > > ====================================================
> > > > 
> > > > Summary
> > > > -------
> > > > 
> > > >   **FAILURE**
> > > > 
> > > >   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
> > > >   verified manually.
> > > >   
> > > >   If you think the reported changes have nothing to do with the changes
> > > >   introduced in Patchwork_14725_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_14725_full:
> > > > 
> > > > ### IGT changes ###
> > > > 
> > > > #### Possible regressions ####
> > > > 
> > > >   * igt@gem_eio@in-flight-1us:
> > > >     - shard-snb:          [PASS][1] -> [FAIL][2]
> > > >    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
> > > >    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> > > > 
> > > >   * igt@kms_plane@pixel-format-pipe-a-planes:
> > > >     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
> > > >    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
> > > >    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html
> > > 
> > > IGT-Version: 1.24-ge501741f
> > > ...
> > > Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
> > > (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)
> > > 
> > > DRM_FORMAT_ARGB2101010 =  0x30335241
> > > IGT_FORMAT_FLOAT = 0x78464749
> > > 
> > > { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
> > >   .pixman_id = PIXMAN_a2r10g10b10,
> > > 
> > > { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
> > >   .pixman_id = PIXMAN_rgba_float,
> > > 
> > > if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> > >     (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> > > 	cnvert_pixman(cvt);
> > > 	return;
> > > ...
> > > igt_assert_f(false, "Conversion not implemented ...);
> > > 
> > > So wtf?
> > > 
> > > Are we somehow compiling igt with an old pixman causing
> > >  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
> > >  #define PIXMAN_rgba_float PIXMAN_invalid
> > >  #endif
> > > to happen?
> > 
> > oof, seems like the building machine got downgraded somehow
> > 
> > ci-worker1:~$ dpkg -l '*pixman*'
> > Desired=Unknown/Install/Remove/Purge/Hold
> > | Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
> > |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
> > ||/ Name                                            Version                      Architecture                 Description
> > +++-===============================================-============================-============================-===================================================================================================
> > ii  libpixman-1-0:amd64                             0.34.0-2                     amd64                        pixel-manipulation library for X and cairo
> > ii  libpixman-1-dev:amd64                           0.34.0-2                     amd64                        pixel-manipulation library for X and cairo (development files)
> > 
> > that's bad...
> > 
> > > But the reference run shows it testing all the fancy YUV formats so
> > > I don't think that can be the case.
> > 
> > That's the weird bit...
> > 
> > Anyway the building machine needs updating and apt-mark hold.
> > This can cause fallout and we need to file bugs to limit the noise.
> > 
> > There is quite some queue right now, but hopefully by tomorrow it will
> > be drained. I'll do the necessary updates and force IGT run to see what
> > is going to happen in the morning. Then I'll rerun this series.
> > 
> 
> 
> I don't think the builder ever had a higher version.
> 
> The fancy YUV formats work because the runtime lib is new enough, and
> the build-time checks for those are as such:
> 
> #if CAIRO_VERSION < CAIRO_VERSION_ENCODE(1, 17, 2)
> /*
>  * We need cairo 1.17.2 to use HDR formats, but the only thing added is a value
>  * to cairo_format_t.
>  *
>  * To prevent going outside the enum, make cairo_format_t an int and define
>  * ourselves.
> */
> 
> #define	CAIRO_FORMAT_RGB96F (6)
> #define	CAIRO_FORMAT_RGBA128F (7)
> #define	cairo_format_t int
> #endif
> 
> 
>   and
> 
> 
> igt_skip_on_f(status == CAIRO_STATUS_INVALID_FORMAT &&
> 	      cairo_version() < CAIRO_VERSION_ENCODE(1, 17, 2),
> 	      "Cairo version too old, need 1.17.2, have %s\n",
> 	      cairo_version_string());
> 
> igt_skip_on_f(status == CAIRO_STATUS_NO_MEMORY &&
> 	      pixman_version() < PIXMAN_VERSION_ENCODE(0, 36, 0),
> 	      "Pixman version too old, need 0.36.0, have %s\n",
> 	      pixman_version_string());
> 
> 
> 
> 
> In other words, the backwards compatibility for the fancy YUV formats
> at build-time was easy to sneak in by defining some values, is that
> possible to do with the 10bpc stuff? PIXMAN_rgba_float seems to be
> just PIXMAN_FORMAT_BYTE(128, PIXMAN_TYPE_RGBA_FLOAT,32,32,32,32),
> where PIXMAN_TYPE_RGBA_FLOAT is 11.

Hmm, yeah I suppose I can try to go that route instead.
Arkadiusz Hiler Oct. 15, 2019, 12:08 p.m. UTC | #6 
On Tue, Oct 15, 2019 at 12:25:59PM +0300, Petri Latvala wrote:
> On Tue, Oct 15, 2019 at 09:41:20AM +0300, Arkadiusz Hiler wrote:
> > On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 09, 2019 at 09:12:23PM -0000, Patchwork wrote:
> > > > == Series Details ==
> > > > 
> > > > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
> > > > URL   : https://patchwork.freedesktop.org/series/67741/
> > > > State : failure
> > > > 
> > > > == Summary ==
> > > > 
> > > > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > > > ====================================================
> > > > 
> > > > Summary
> > > > -------
> > > > 
> > > >   **FAILURE**
> > > > 
> > > >   Serious unknown changes coming with Patchwork_14725_full absolutely need to be
> > > >   verified manually.
> > > >   
> > > >   If you think the reported changes have nothing to do with the changes
> > > >   introduced in Patchwork_14725_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_14725_full:
> > > > 
> > > > ### IGT changes ###
> > > > 
> > > > #### Possible regressions ####
> > > > 
> > > >   * igt@gem_eio@in-flight-1us:
> > > >     - shard-snb:          [PASS][1] -> [FAIL][2]
> > > >    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_eio@in-flight-1us.html
> > > >    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_eio@in-flight-1us.html
> > > > 
> > > >   * igt@kms_plane@pixel-format-pipe-a-planes:
> > > >     - shard-iclb:         [PASS][3] -> [FAIL][4] +13 similar issues
> > > >    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_plane@pixel-format-pipe-a-planes.html
> > > >    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes.html
> > > 
> > > IGT-Version: 1.24-ge501741f
> > > ...
> > > Testing format AR30(0x30335241) / modifier 0x100000000000003 on A.0
> > > (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 0x30335241 to 0x78464749)
> > > 
> > > DRM_FORMAT_ARGB2101010 =  0x30335241
> > > IGT_FORMAT_FLOAT = 0x78464749
> > > 
> > > { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
> > >   .pixman_id = PIXMAN_a2r10g10b10,
> > > 
> > > { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
> > >   .pixman_id = PIXMAN_rgba_float,
> > > 
> > > if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> > >     (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> > > 	cnvert_pixman(cvt);
> > > 	return;
> > > ...
> > > igt_assert_f(false, "Conversion not implemented ...);
> > > 
> > > So wtf?
> > > 
> > > Are we somehow compiling igt with an old pixman causing
> > >  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
> > >  #define PIXMAN_rgba_float PIXMAN_invalid
> > >  #endif
> > > to happen?
> > 
> > oof, seems like the building machine got downgraded somehow
> > 
> > ci-worker1:~$ dpkg -l '*pixman*'
> > Desired=Unknown/Install/Remove/Purge/Hold
> > | Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
> > |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
> > ||/ Name                                            Version                      Architecture                 Description
> > +++-===============================================-============================-============================-===================================================================================================
> > ii  libpixman-1-0:amd64                             0.34.0-2                     amd64                        pixel-manipulation library for X and cairo
> > ii  libpixman-1-dev:amd64                           0.34.0-2                     amd64                        pixel-manipulation library for X and cairo (development files)
> > 
> > that's bad...
> > 
> > > But the reference run shows it testing all the fancy YUV formats so
> > > I don't think that can be the case.
> > 
> > That's the weird bit...
> > 
> > Anyway the building machine needs updating and apt-mark hold.
> > This can cause fallout and we need to file bugs to limit the noise.
> > 
> > There is quite some queue right now, but hopefully by tomorrow it will
> > be drained. I'll do the necessary updates and force IGT run to see what
> > is going to happen in the morning. Then I'll rerun this series.
> > 
> 
> 
> I don't think the builder ever had a higher version.
> 
> The fancy YUV formats work because the runtime lib is new enough, and
> the build-time checks for those are as such:
> 
> #if CAIRO_VERSION < CAIRO_VERSION_ENCODE(1, 17, 2)
> /*
>  * We need cairo 1.17.2 to use HDR formats, but the only thing added is a value
>  * to cairo_format_t.
>  *
>  * To prevent going outside the enum, make cairo_format_t an int and define
>  * ourselves.
> */
> 
> #define	CAIRO_FORMAT_RGB96F (6)
> #define	CAIRO_FORMAT_RGBA128F (7)
> #define	cairo_format_t int
> #endif
> 
> 
>   and
> 
> 
> igt_skip_on_f(status == CAIRO_STATUS_INVALID_FORMAT &&
> 	      cairo_version() < CAIRO_VERSION_ENCODE(1, 17, 2),
> 	      "Cairo version too old, need 1.17.2, have %s\n",
> 	      cairo_version_string());
> 
> igt_skip_on_f(status == CAIRO_STATUS_NO_MEMORY &&
> 	      pixman_version() < PIXMAN_VERSION_ENCODE(0, 36, 0),
> 	      "Pixman version too old, need 0.36.0, have %s\n",
> 	      pixman_version_string());

I am not a fan of this approach as we nest skips pretty deep within
functions that have neither 'require' nor 'skip' in the name. The
issuing longjmp() can cause surprise short-circuiting of tests iterating
through multiple formats...

Some of that would be solved by the "dynamic subsubtests", but the
semantics still feels off.

> In other words, the backwards compatibility for the fancy YUV formats
> at build-time was easy to sneak in by defining some values, is that
> possible to do with the 10bpc stuff? PIXMAN_rgba_float seems to be
> just PIXMAN_FORMAT_BYTE(128, PIXMAN_TYPE_RGBA_FLOAT,32,32,32,32),
> where PIXMAN_TYPE_RGBA_FLOAT is 11.

Yeah, let it be for now unless someone finds time to do the necessary
reworks :-P
Shankar, Uma Oct. 29, 2019, 9:08 a.m. UTC | #7 
>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
>Sent: Tuesday, October 8, 2019 9:45 PM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH 1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-
>BDW sprites
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>SNB-BDW support 10:10:10 formats on the sprite planes. Let's expose them.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_sprite.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
>b/drivers/gpu/drm/i915/display/intel_sprite.c
>index 633fa8069348..90b0e65420a5 100644
>--- a/drivers/gpu/drm/i915/display/intel_sprite.c
>+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>@@ -1054,6 +1054,12 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state
>*crtc_state,
> 	case DRM_FORMAT_XRGB8888:
> 		sprctl |= SPRITE_FORMAT_RGBX888;
> 		break;
>+	case DRM_FORMAT_XBGR2101010:
>+		sprctl |= SPRITE_FORMAT_RGBX101010 |
>SPRITE_RGB_ORDER_RGBX;
>+		break;
>+	case DRM_FORMAT_XRGB2101010:
>+		sprctl |= SPRITE_FORMAT_RGBX101010;
>+		break;
> 	case DRM_FORMAT_YUYV:
> 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
> 		break;
>@@ -1288,6 +1294,12 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state
>*crtc_state,
> 	case DRM_FORMAT_XRGB8888:
> 		dvscntr |= DVS_FORMAT_RGBX888;
> 		break;
>+	case DRM_FORMAT_XBGR2101010:
>+		dvscntr |= DVS_FORMAT_RGBX101010 | DVS_RGB_ORDER_XBGR;
>+		break;
>+	case DRM_FORMAT_XRGB2101010:
>+		dvscntr |= DVS_FORMAT_RGBX101010;
>+		break;
> 	case DRM_FORMAT_YUYV:
> 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
> 		break;
>@@ -1983,6 +1995,8 @@ static const u64 i9xx_plane_format_modifiers[] = {  static
>const u32 snb_plane_formats[] = {
> 	DRM_FORMAT_XBGR8888,
> 	DRM_FORMAT_XRGB8888,
>+	DRM_FORMAT_XRGB2101010,
>+	DRM_FORMAT_XBGR2101010,

The order seems inconsistent with 8888 but I guess you have a patch fixing the
ordering, so should be ok. 

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> 	DRM_FORMAT_YUYV,
> 	DRM_FORMAT_YVYU,
> 	DRM_FORMAT_UYVY,
>@@ -2193,6 +2207,8 @@ static bool snb_sprite_format_mod_supported(struct
>drm_plane *_plane,
> 	switch (format) {
> 	case DRM_FORMAT_XRGB8888:
> 	case DRM_FORMAT_XBGR8888:
>+	case DRM_FORMAT_XRGB2101010:
>+	case DRM_FORMAT_XBGR2101010:
> 	case DRM_FORMAT_YUYV:
> 	case DRM_FORMAT_YVYU:
> 	case DRM_FORMAT_UYVY:
>--
>2.21.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Juha-Pekka Heikkila Oct. 29, 2019, 8:15 p.m. UTC | #8 
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

On 8.10.2019 19.14, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> SNB-BDW support 10:10:10 formats on the sprite planes. Let's expose
> them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_sprite.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 633fa8069348..90b0e65420a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -1054,6 +1054,12 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
>   	case DRM_FORMAT_XRGB8888:
>   		sprctl |= SPRITE_FORMAT_RGBX888;
>   		break;
> +	case DRM_FORMAT_XBGR2101010:
> +		sprctl |= SPRITE_FORMAT_RGBX101010 | SPRITE_RGB_ORDER_RGBX;
> +		break;
> +	case DRM_FORMAT_XRGB2101010:
> +		sprctl |= SPRITE_FORMAT_RGBX101010;
> +		break;
>   	case DRM_FORMAT_YUYV:
>   		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
>   		break;
> @@ -1288,6 +1294,12 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
>   	case DRM_FORMAT_XRGB8888:
>   		dvscntr |= DVS_FORMAT_RGBX888;
>   		break;
> +	case DRM_FORMAT_XBGR2101010:
> +		dvscntr |= DVS_FORMAT_RGBX101010 | DVS_RGB_ORDER_XBGR;
> +		break;
> +	case DRM_FORMAT_XRGB2101010:
> +		dvscntr |= DVS_FORMAT_RGBX101010;
> +		break;
>   	case DRM_FORMAT_YUYV:
>   		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
>   		break;
> @@ -1983,6 +1995,8 @@ static const u64 i9xx_plane_format_modifiers[] = {
>   static const u32 snb_plane_formats[] = {
>   	DRM_FORMAT_XBGR8888,
>   	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
>   	DRM_FORMAT_YUYV,
>   	DRM_FORMAT_YVYU,
>   	DRM_FORMAT_UYVY,
> @@ -2193,6 +2207,8 @@ static bool snb_sprite_format_mod_supported(struct drm_plane *_plane,
>   	switch (format) {
>   	case DRM_FORMAT_XRGB8888:
>   	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_XBGR2101010:
>   	case DRM_FORMAT_YUYV:
>   	case DRM_FORMAT_YVYU:
>   	case DRM_FORMAT_UYVY:
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 633fa8069348..90b0e65420a5 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -1054,6 +1054,12 @@  static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XRGB8888:
 		sprctl |= SPRITE_FORMAT_RGBX888;
 		break;
+	case DRM_FORMAT_XBGR2101010:
+		sprctl |= SPRITE_FORMAT_RGBX101010 | SPRITE_RGB_ORDER_RGBX;
+		break;
+	case DRM_FORMAT_XRGB2101010:
+		sprctl |= SPRITE_FORMAT_RGBX101010;
+		break;
 	case DRM_FORMAT_YUYV:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
 		break;
@@ -1288,6 +1294,12 @@  static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XRGB8888:
 		dvscntr |= DVS_FORMAT_RGBX888;
 		break;
+	case DRM_FORMAT_XBGR2101010:
+		dvscntr |= DVS_FORMAT_RGBX101010 | DVS_RGB_ORDER_XBGR;
+		break;
+	case DRM_FORMAT_XRGB2101010:
+		dvscntr |= DVS_FORMAT_RGBX101010;
+		break;
 	case DRM_FORMAT_YUYV:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
 		break;
@@ -1983,6 +1995,8 @@  static const u64 i9xx_plane_format_modifiers[] = {
 static const u32 snb_plane_formats[] = {
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
@@ -2193,6 +2207,8 @@  static bool snb_sprite_format_mod_supported(struct drm_plane *_plane,
 	switch (format) {
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY: