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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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