[PATCH] drm/i915: Fix skl srckey mask bits

[PATCH] drm/i915: Fix skl srckey mask bits

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're incorrectly masking off the R/V channel enable bit from
KEYMSK. Fix it up.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b02d3d9809e3..cd42e81f8a90 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
 
 	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
 
-	keymsk = key->channel_mask & 0x3ffffff;
+	keymsk = key->channel_mask & 0x7ffffff;
 	if (alpha < 0xff)
 		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
 
-- 
2.19.2

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

✓ Fi.CI.BAT: success for drm/i915: Fix skl srckey mask bits

[Patchwork]

== Series Details ==

Series: drm/i915: Fix skl srckey mask bits
URL   : https://patchwork.freedesktop.org/series/55744/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5486 -> Patchwork_12045
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55744/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12045 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#108767]

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6700hq:      PASS -> DMESG-WARN [fdo#105998]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-bsw-n3050:       FAIL [fdo#100368] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (43 -> 41)
------------------------------

  Additional (2): fi-byt-clapper fi-pnv-d510 
  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-------------

    * Linux: CI_DRM_5486 -> Patchwork_12045

  CI_DRM_5486: 186ccd79d43ff0c930c7211e24ba5afa78c6594e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12045: 93c1c37b7d5fe620aa660d8893bff83a90265839 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

93c1c37b7d5f drm/i915: Fix skl srckey mask bits

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12045/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Matt Roper]

On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're incorrectly masking off the R/V channel enable bit from
> KEYMSK. Fix it up.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

The "srckey" terminology in the headline is a bit confusing since the
channel mask applies to both "destination colorkey" and "source
colorkey" behavior.  Otherwise,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Out of curiousity, has there ever been a userspace consumer of source
colorkey?  I know SNA uses destination colorkey (for Xv), but not source
colorkey.  And afaik, UXA never did any colorkeying through the sprite
interface at all.


Matt

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index b02d3d9809e3..cd42e81f8a90 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
>  
>  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
>  
> -	keymsk = key->channel_mask & 0x3ffffff;
> +	keymsk = key->channel_mask & 0x7ffffff;
>  	if (alpha < 0xff)
>  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
>  
> -- 
> 2.19.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Fix skl srckey mask bits

[Patchwork]

== Series Details ==

Series: drm/i915: Fix skl srckey mask bits
URL   : https://patchwork.freedesktop.org/series/55744/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5486_full -> Patchwork_12045_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12045_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@runner@aborted}:
    - shard-kbl:          ( 5 FAIL ) -> ( 4 FAIL )

  
Known issues
------------

  Here are the changes found in Patchwork_12045_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@gem_eio@in-flight-suspend:
    - shard-snb:          PASS -> FAIL [fdo#103375]

  * igt@gem_eio@reset-stress:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          PASS -> FAIL [fdo#106641]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_color@pipe-c-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-glk:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          PASS -> FAIL [fdo#103060]

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_setmode@basic:
    - shard-hsw:          PASS -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-kbl:          PASS -> FAIL [fdo#105010]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-immediate:
    - shard-kbl:          DMESG-FAIL -> PASS

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_flip@2x-flip-vs-modeset-interruptible:
    - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-apl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5486 -> Patchwork_12045

  CI_DRM_5486: 186ccd79d43ff0c930c7211e24ba5afa78c6594e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12045: 93c1c37b7d5fe620aa660d8893bff83a90265839 @ 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_12045/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Ville Syrjälä]

On Fri, Jan 25, 2019 at 01:54:47PM -0800, Matt Roper wrote:
> On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We're incorrectly masking off the R/V channel enable bit from
> > KEYMSK. Fix it up.
> > 
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The "srckey" terminology in the headline is a bit confusing since the
> channel mask applies to both "destination colorkey" and "source
> colorkey" behavior.

Does it? IIRC it used to be only for source keying. I guess I'll have to
try it for reals.

> Otherwise,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> Out of curiousity, has there ever been a userspace consumer of source
> colorkey?  I know SNA uses destination colorkey (for Xv), but not source
> colorkey.  And afaik, UXA never did any colorkeying through the sprite
> interface at all.

No, I don't think we have any userspace for this.

> 
> 
> Matt
> 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index b02d3d9809e3..cd42e81f8a90 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
> >  
> >  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> >  
> > -	keymsk = key->channel_mask & 0x3ffffff;
> > +	keymsk = key->channel_mask & 0x7ffffff;
> >  	if (alpha < 0xff)
> >  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> >  
> > -- 
> > 2.19.2
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Ville Syrjälä]

On Tue, Jan 29, 2019 at 04:33:15PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 25, 2019 at 01:54:47PM -0800, Matt Roper wrote:
> > On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We're incorrectly masking off the R/V channel enable bit from
> > > KEYMSK. Fix it up.
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The "srckey" terminology in the headline is a bit confusing since the
> > channel mask applies to both "destination colorkey" and "source
> > colorkey" behavior.
> 
> Does it? IIRC it used to be only for source keying. I guess I'll have to
> try it for reals.

Still seems to affect only the source key.

> 
> > Otherwise,
> > 
> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > 
> > Out of curiousity, has there ever been a userspace consumer of source
> > colorkey?  I know SNA uses destination colorkey (for Xv), but not source
> > colorkey.  And afaik, UXA never did any colorkeying through the sprite
> > interface at all.
> 
> No, I don't think we have any userspace for this.
> 
> > 
> > 
> > Matt
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index b02d3d9809e3..cd42e81f8a90 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
> > >  
> > >  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> > >  
> > > -	keymsk = key->channel_mask & 0x3ffffff;
> > > +	keymsk = key->channel_mask & 0x7ffffff;
> > >  	if (alpha < 0xff)
> > >  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > >  
> > > -- 
> > > 2.19.2
> > > 
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > IoTG Platform Enabling & Development
> > Intel Corporation
> > (916) 356-2795
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Ville Syrjälä]

On Tue, Jan 29, 2019 at 04:49:19PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 29, 2019 at 04:33:15PM +0200, Ville Syrjälä wrote:
> > On Fri, Jan 25, 2019 at 01:54:47PM -0800, Matt Roper wrote:
> > > On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We're incorrectly masking off the R/V channel enable bit from
> > > > KEYMSK. Fix it up.
> > > > 
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The "srckey" terminology in the headline is a bit confusing since the
> > > channel mask applies to both "destination colorkey" and "source
> > > colorkey" behavior.
> > 
> > Does it? IIRC it used to be only for source keying. I guess I'll have to
> > try it for reals.
> 
> Still seems to affect only the source key.

And with that confirmed I went ahead and pushed this as is. Thanks for
the review.

> 
> > 
> > > Otherwise,
> > > 
> > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > > 
> > > Out of curiousity, has there ever been a userspace consumer of source
> > > colorkey?  I know SNA uses destination colorkey (for Xv), but not source
> > > colorkey.  And afaik, UXA never did any colorkeying through the sprite
> > > interface at all.
> > 
> > No, I don't think we have any userspace for this.
> > 
> > > 
> > > 
> > > Matt
> > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index b02d3d9809e3..cd42e81f8a90 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
> > > >  
> > > >  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> > > >  
> > > > -	keymsk = key->channel_mask & 0x3ffffff;
> > > > +	keymsk = key->channel_mask & 0x7ffffff;
> > > >  	if (alpha < 0xff)
> > > >  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > > >  
> > > > -- 
> > > > 2.19.2
> > > > 
> > > 
> > > -- 
> > > Matt Roper
> > > Graphics Software Engineer
> > > IoTG Platform Enabling & Development
> > > Intel Corporation
> > > (916) 356-2795
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> 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

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Matt Roper]

On Tue, Jan 29, 2019 at 04:49:19PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 29, 2019 at 04:33:15PM +0200, Ville Syrjälä wrote:
> > On Fri, Jan 25, 2019 at 01:54:47PM -0800, Matt Roper wrote:
> > > On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We're incorrectly masking off the R/V channel enable bit from
> > > > KEYMSK. Fix it up.
> > > > 
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The "srckey" terminology in the headline is a bit confusing since the
> > > channel mask applies to both "destination colorkey" and "source
> > > colorkey" behavior.
> > 
> > Does it? IIRC it used to be only for source keying. I guess I'll have to
> > try it for reals.
> 
> Still seems to affect only the source key.

Hmm, strange.  I don't see any mention of this only applying to source
color key in the bspec (neither PLANE_KEYVAL or PLANE_KEYMSK
descriptions mention it).  I also notice that SNA explicitly sets the
channel mask (including the 0x7 << 24 bits), even though it only uses
destination color keying.

Is this documented somewhere that we're overlooking, or should we file a
bspec defect to get it added?


Matt

> 
> > 
> > > Otherwise,
> > > 
> > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > > 
> > > Out of curiousity, has there ever been a userspace consumer of source
> > > colorkey?  I know SNA uses destination colorkey (for Xv), but not source
> > > colorkey.  And afaik, UXA never did any colorkeying through the sprite
> > > interface at all.
> > 
> > No, I don't think we have any userspace for this.
> > 
> > > 
> > > 
> > > Matt
> > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index b02d3d9809e3..cd42e81f8a90 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
> > > >  
> > > >  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> > > >  
> > > > -	keymsk = key->channel_mask & 0x3ffffff;
> > > > +	keymsk = key->channel_mask & 0x7ffffff;
> > > >  	if (alpha < 0xff)
> > > >  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > > >  
> > > > -- 
> > > > 2.19.2
> > > > 
> > > 
> > > -- 
> > > Matt Roper
> > > Graphics Software Engineer
> > > IoTG Platform Enabling & Development
> > > Intel Corporation
> > > (916) 356-2795
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix skl srckey mask bits

[Ville Syrjälä]

On Wed, Jan 30, 2019 at 10:25:22AM -0800, Matt Roper wrote:
> On Tue, Jan 29, 2019 at 04:49:19PM +0200, Ville Syrjälä wrote:
> > On Tue, Jan 29, 2019 at 04:33:15PM +0200, Ville Syrjälä wrote:
> > > On Fri, Jan 25, 2019 at 01:54:47PM -0800, Matt Roper wrote:
> > > > On Fri, Jan 25, 2019 at 08:38:46PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > We're incorrectly masking off the R/V channel enable bit from
> > > > > KEYMSK. Fix it up.
> > > > > 
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > > Fixes: b20815255693 ("drm/i915: Add plane alpha blending support, v2.")
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > The "srckey" terminology in the headline is a bit confusing since the
> > > > channel mask applies to both "destination colorkey" and "source
> > > > colorkey" behavior.
> > > 
> > > Does it? IIRC it used to be only for source keying. I guess I'll have to
> > > try it for reals.
> > 
> > Still seems to affect only the source key.
> 
> Hmm, strange.  I don't see any mention of this only applying to source
> color key in the bspec (neither PLANE_KEYVAL or PLANE_KEYMSK
> descriptions mention it).  I also notice that SNA explicitly sets the
> channel mask (including the 0x7 << 24 bits), even though it only uses
> destination color keying.
> 
> Is this documented somewhere that we're overlooking, or should we file a
> bspec defect to get it added?

It used to be documented. Eg. g4x docs say:

"26 V/R Channel Source Key Enable: Specifies the source color key enable for the V/Red channel.
 25 Y/G Channel Source Key Enable: Specifies the source color key enable for the Y/Green channel.
 24 U/B Channel Source Key Enable: Specifies the source color key enable for the U/Blue channel
 23:16 R mask Dest Key Value: Specifies the destination color key mask for the sprite R channel.
 15:8 G mask Dest Key Value: Specifies the destination color key mask for the sprite G channel.
 7:0 B mask Dest Key Value: Specifies the destination color key mask for the sprite B channel."

Curiously it also says:
"Note that source key and destination key are mutually exclusive modes of
 operation, they can not be used simultaneously. For the function that is
 not enabled, the associated bits in this register should be programmed to
 zeroes."
which we are clearly not following. But I don't know of any issue that
has caused.

Prior to g4x keying was way more confusing as there were a lot more ways
the planes could interact. I think skl sort of went back to the pre-g4x
style by moving the destination keying from the "sprite" to the
"primary" and so it's more like the "src key window" mode of old.
Curiously skl also has that mode, not quite sure what the difference is
between the two tbh. Also vlv/chv are totally in the pre-g4x camp, and
thus implementing destination keying on those is a bit more complicated
(https://patchwork.freedesktop.org/series/43902/).

> 
> 
> Matt
> 
> > 
> > > 
> > > > Otherwise,
> > > > 
> > > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > > > 
> > > > Out of curiousity, has there ever been a userspace consumer of source
> > > > colorkey?  I know SNA uses destination colorkey (for Xv), but not source
> > > > colorkey.  And afaik, UXA never did any colorkeying through the sprite
> > > > interface at all.
> > > 
> > > No, I don't think we have any userspace for this.
> > > 
> > > > 
> > > > 
> > > > Matt
> > > > 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > index b02d3d9809e3..cd42e81f8a90 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > @@ -493,7 +493,7 @@ skl_program_plane(struct intel_plane *plane,
> > > > >  
> > > > >  	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> > > > >  
> > > > > -	keymsk = key->channel_mask & 0x3ffffff;
> > > > > +	keymsk = key->channel_mask & 0x7ffffff;
> > > > >  	if (alpha < 0xff)
> > > > >  		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > > > >  
> > > > > -- 
> > > > > 2.19.2
> > > > > 
> > > > 
> > > > -- 
> > > > Matt Roper
> > > > Graphics Software Engineer
> > > > IoTG Platform Enabling & Development
> > > > Intel Corporation
> > > > (916) 356-2795
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx