[PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Jani Nikula]

skylake_update_primary_plane() did not handle all pixel formats returned
by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

This is purely cargo culting to avoid the BUG.
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3fe95982be93..cede05256d56 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	case DRM_FORMAT_XRGB8888:
 		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
 		break;
+	case DRM_FORMAT_ARGB8888:
+		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
+		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+		break;
 	case DRM_FORMAT_XBGR8888:
 		plane_ctl |= PLANE_CTL_ORDER_RGBX;
 		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
 		break;
+	case DRM_FORMAT_ABGR8888:
+		plane_ctl |= PLANE_CTL_ORDER_RGBX;
+		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
+		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+		break;
 	case DRM_FORMAT_XRGB2101010:
 		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
 		break;
-- 
2.1.4

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

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Ville Syrjälä]

On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
> skylake_update_primary_plane() did not handle all pixel formats returned
> by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> This is purely cargo culting to avoid the BUG.
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3fe95982be93..cede05256d56 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	case DRM_FORMAT_XRGB8888:
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>  		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;

We don't do alpha blending yet properly so we should just ignore alpha
for now. And someone should rip out that bit from skl_update_plane() as
well.

Also we seem to be missing more formats here than just these two. hint:
intel_primary_formats_gen4[]

> +		break;
>  	case DRM_FORMAT_XBGR8888:
>  		plane_ctl |= PLANE_CTL_ORDER_RGBX;
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>  		break;
> +	case DRM_FORMAT_ABGR8888:
> +		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
>  	case DRM_FORMAT_XRGB2101010:
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
>  		break;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Damien Lespiau]

On Tue, Feb 10, 2015 at 01:43:39PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
> > skylake_update_primary_plane() did not handle all pixel formats returned
> > by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > ---
> > 
> > This is purely cargo culting to avoid the BUG.
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3fe95982be93..cede05256d56 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> >  	case DRM_FORMAT_XRGB8888:
> >  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> >  		break;
> > +	case DRM_FORMAT_ARGB8888:
> > +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> > +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> 
> We don't do alpha blending yet properly so we should just ignore alpha
> for now. And someone should rip out that bit from skl_update_plane() as
> well.

We currently expose ARGB planes for VLV. Looking at the VLV Diplay
Cluster HAS the blending done in VLV (and so CHV I'm guessing?) is 

	src + (1 - src_a) * dst

Clearly suitable for pre-multiplied framebuffers (it's also stated so).
So, we already expose a default blendig mode suitable for pre-multiplied
FBs. This would be just doing the same and provide a (IMHO sensible)
default for fbs with alpha.

So we could go with this? maybe?

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

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Ville Syrjälä]

On Mon, Feb 16, 2015 at 02:22:20PM +0000, Damien Lespiau wrote:
> On Tue, Feb 10, 2015 at 01:43:39PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
> > > skylake_update_primary_plane() did not handle all pixel formats returned
> > > by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > 
> > > ---
> > > 
> > > This is purely cargo culting to avoid the BUG.
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3fe95982be93..cede05256d56 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> > >  	case DRM_FORMAT_XRGB8888:
> > >  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> > >  		break;
> > > +	case DRM_FORMAT_ARGB8888:
> > > +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> > > +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> > 
> > We don't do alpha blending yet properly so we should just ignore alpha
> > for now. And someone should rip out that bit from skl_update_plane() as
> > well.
> 
> We currently expose ARGB planes for VLV. Looking at the VLV Diplay
> Cluster HAS the blending done in VLV (and so CHV I'm guessing?) is 
> 
> 	src + (1 - src_a) * dst
> 
> Clearly suitable for pre-multiplied framebuffers (it's also stated so).
> So, we already expose a default blendig mode suitable for pre-multiplied
> FBs. This would be just doing the same and provide a (IMHO sensible)
> default for fbs with alpha.
> 
> So we could go with this? maybe?

I thought we had a separate bit for actually enabling alpha blending.
But I guess we don't? If so, we've maybe made a bit of a mess of
things already. And anyway we've made an even bigger mess by exposing
cursor planes before we have any alpha blending props.

But yeah if we make the default blending mode
'1*Sc + (1-Sa)*Dc, no extra premultiplication' for any alpha
blending capable plane, it should mostly come out all right... I hope.
And for any non alpha capable planes we obviously make the default
(and only supported value) '1*Sc + 0*Dc'. If anyone has been using
the A formats with non-alpha blending capable planes, well they
already get something a bit iffty as the output when they
feed it premultiplied data (assuming alpha < 1.0 obviously), and
that wouldn't change with the addition of the alpha blending props.

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

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5742
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV              +4-1              275/283              278/283
ILK                                  310/315              310/315
SNB              +2                 320/346              322/346
IVB                                  380/384              380/384
BYT                                  296/296              296/296
HSW              +3                 422/428              425/428
BDW                                  318/333              318/333
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_coherency-sync      CRASH(1, M7)PASS(1, M7)      PASS(1, M7)
 PNV  igt_gem_userptr_blits_coherency-unsync      CRASH(1, M7)PASS(1, M7)      PASS(1, M7)
 PNV  igt_gem_userptr_blits_create-destroy-sync      NRUN(1, M7)PASS(1, M7)      PASS(1, M7)
*PNV  igt_gen3_render_tiledx_blits      TIMEOUT(1, M7)PASS(1, M7)      FAIL(1, M7)
 PNV  igt_gen3_render_tiledy_blits      FAIL(1, M7)PASS(1, M7)      PASS(1, M7)
*PNV  igt_gem_tiled_pread_pwrite      PASS(2, M7)      FAIL(1, M7)
*SNB  igt_kms_flip_bo-too-big      BLACKLIST(1, M35)      PASS(1, M35)
*SNB  igt_kms_flip_bo-too-big-interruptible      BLACKLIST(1, M35)      PASS(1, M35)
*HSW  igt_kms_flip_bo-too-big      BLACKLIST(1, M40)      PASS(1, M40)
*HSW  igt_kms_flip_bo-too-big-interruptible      BLACKLIST(1, M40)      PASS(1, M40)
 HSW  igt_kms_flip_plain-flip-fb-recreate-interruptible      TIMEOUT(1, M40)PASS(1, M40)      PASS(1, M40)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Damien Lespiau]

On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
> skylake_update_primary_plane() did not handle all pixel formats returned
> by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Given the discussion with Ville, it's quite likely we'll default to
alpha blending for pre-multiplied fbs (for plane supporting alpha), even
with the blending properties added. In that context, we can provide a
single, fixed, (but useful) blending mode before we get to implement the
full thing. So:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
> 
> This is purely cargo culting to avoid the BUG.
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3fe95982be93..cede05256d56 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	case DRM_FORMAT_XRGB8888:
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>  		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
>  	case DRM_FORMAT_XBGR8888:
>  		plane_ctl |= PLANE_CTL_ORDER_RGBX;
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>  		break;
> +	case DRM_FORMAT_ABGR8888:
> +		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
>  	case DRM_FORMAT_XRGB2101010:
>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
>  		break;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Jani Nikula]

On Mon, 16 Feb 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
> On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
>> skylake_update_primary_plane() did not handle all pixel formats returned
>> by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Given the discussion with Ville, it's quite likely we'll default to
> alpha blending for pre-multiplied fbs (for plane supporting alpha), even
> with the blending properties added. In that context, we can provide a
> single, fixed, (but useful) blending mode before we get to implement the
> full thing. So:
>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Pushed to drm-intel-fixes, thanks for the review.

BR,
Jani.


>
> -- 
> Damien
>
>> ---
>> 
>> This is purely cargo culting to avoid the BUG.
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3fe95982be93..cede05256d56 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2751,10 +2751,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>>  	case DRM_FORMAT_XRGB8888:
>>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>>  		break;
>> +	case DRM_FORMAT_ARGB8888:
>> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>> +		break;
>>  	case DRM_FORMAT_XBGR8888:
>>  		plane_ctl |= PLANE_CTL_ORDER_RGBX;
>>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>>  		break;
>> +	case DRM_FORMAT_ABGR8888:
>> +		plane_ctl |= PLANE_CTL_ORDER_RGBX;
>> +		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
>> +		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>> +		break;
>>  	case DRM_FORMAT_XRGB2101010:
>>  		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
>>  		break;
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Ville Syrjälä]

On Mon, Feb 23, 2015 at 03:15:58PM +0200, Jani Nikula wrote:
> On Mon, 16 Feb 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
> > On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
> >> skylake_update_primary_plane() did not handle all pixel formats returned
> >> by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
> >> 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > Given the discussion with Ville, it's quite likely we'll default to
> > alpha blending for pre-multiplied fbs (for plane supporting alpha), even
> > with the blending properties added. In that context, we can provide a
> > single, fixed, (but useful) blending mode before we get to implement the
> > full thing. So:
> >
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> Pushed to drm-intel-fixes, thanks for the review.

Well, the patch still isn't quite right. intel_primary_formats_gen4[]
defines which formats the core will let slip through into the driver,
and even with this patch we're not handling them all.

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

Re: [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()

[Jani Nikula]

On Tue, 24 Feb 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Feb 23, 2015 at 03:15:58PM +0200, Jani Nikula wrote:
>> On Mon, 16 Feb 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
>> > On Tue, Feb 10, 2015 at 01:15:49PM +0200, Jani Nikula wrote:
>> >> skylake_update_primary_plane() did not handle all pixel formats returned
>> >> by skl_format_to_fourcc(). Handle alpha similar to skl_update_plane().
>> >> 
>> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89052
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >
>> > Given the discussion with Ville, it's quite likely we'll default to
>> > alpha blending for pre-multiplied fbs (for plane supporting alpha), even
>> > with the blending properties added. In that context, we can provide a
>> > single, fixed, (but useful) blending mode before we get to implement the
>> > full thing. So:
>> >
>> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>> 
>> Pushed to drm-intel-fixes, thanks for the review.
>
> Well, the patch still isn't quite right. intel_primary_formats_gen4[]
> defines which formats the core will let slip through into the driver,
> and even with this patch we're not handling them all.

Blergh, patches welcome. :p

Jani.


>
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx