| Message ID | 1423566949-5319-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 '*'
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?
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.
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>
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
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.
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
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;
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(+)