Re: [PATCH v2] drm/rockchip: Disable blending for win0

[Sean Paul <seanpaul@chromium.org>]

On Wed, Apr 18, 2018 at 02:53:08PM -0700, Kristian Kristensen wrote:
> On Wed, Apr 18, 2018 at 11:23 AM, Sean Paul <seanpaul@chromium.org> wrote:
> 
> > On Wed, Apr 18, 2018 at 10:31:52AM -0700, Kristian H. Kristensen wrote:
> > > Blending win0 with the background color doesn't seem to work
> > > correctly.
> >
> > Did you come to a conclusion about whether this applies across all Rk
> > vops, or
> > just 3399 big?
> >
> 
> It didn't work on either big or little VOP on 3399, but oddly, 3288 seemed
> to work fine. Either way, what the patch is doing is correct and simpler,
> so I'm not sure it makes sense to try to be more specific about when we
> turn off blending.

Yeah, fair enough. We'll need to revisit if we ever support anything more than
pre-mult/black bg anyways, so

Reviewed-by: Sean Paul <seanpaul@chromium.org>

Sean


> 
> Kristian
> 
> 
> > Sean
> >
> > > We only get the background color, no matter the contents of
> > > the win0 framebuffer.  However, blending pre-multiplied color with the
> > > default opaque black default background color is a no-op, so we can
> > > just disable blending to get the correct result.
> > >
> > > Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> > > Cc: Sandy Huang <hjc@rock-chips.com>
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > ---
> > >
> > > v2: Drop CHROMIUM: prefix, rebase on Linus' master
> > >
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > index 53d4afe15278..753a7548da84 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > @@ -76,6 +76,9 @@
> > >  #define VOP_WIN_GET_YRGBADDR(vop, win) \
> > >               vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
> > >
> > > +#define VOP_WIN_TO_INDEX(vop_win) \
> > > +     ((vop_win) - (vop_win)->vop->win)
> > > +
> > >  #define to_vop(x) container_of(x, struct vop, crtc)
> > >  #define to_vop_win(x) container_of(x, struct vop_win, base)
> > >
> > > @@ -708,6 +711,7 @@ static void vop_plane_atomic_update(struct drm_plane
> > *plane,
> > >       dma_addr_t dma_addr;
> > >       uint32_t val;
> > >       bool rb_swap;
> > > +     int win_index = VOP_WIN_TO_INDEX(vop_win);
> > >       int format;
> > >
> > >       /*
> > > @@ -777,7 +781,14 @@ static void vop_plane_atomic_update(struct
> > drm_plane *plane,
> > >       rb_swap = has_rb_swapped(fb->format->format);
> > >       VOP_WIN_SET(vop, win, rb_swap, rb_swap);
> > >
> > > -     if (fb->format->has_alpha) {
> > > +     /*
> > > +      * Blending win0 with the background color doesn't seem to work
> > > +      * correctly. We only get the background color, no matter the
> > contents
> > > +      * of the win0 framebuffer.  However, blending pre-multiplied color
> > > +      * with the default opaque black default background color is a
> > no-op,
> > > +      * so we can just disable blending to get the correct result.
> > > +      */
> > > +     if (fb->format->has_alpha && win_index > 0) {
> > >               VOP_WIN_SET(vop, win, dst_alpha_ctl,
> > >                           DST_FACTOR_M0(ALPHA_SRC_INVERSE));
> > >               val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
> > > --
> > > 2.17.0.484.g0c8726318c-goog
> > >
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel