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

[Sean Paul]

On Tue, Apr 17, 2018 at 09:37:44AM -0400, Sean Paul wrote:
> On Mon, Apr 16, 2018 at 03:22:15PM -0700, Kristian H. Kristensen wrote:
> > 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.
> > 
> > Signed-off-by: Kristian H. Kristensen 
> > Cc: Sandy Huang 
> > Cc: Sean Paul 
> 
> Sandy, when you push this, could you please remove the CHROMIUM: prefix from 
> the
> subject?
> 
> Reviewed-by: Sean Paul 

Sigh. I should read replies before opening my mouth. Looks like you'll need to
rebase on drm-misc-next and fix the compile issue.

Sean

> 
> 
> > ---
> > As per Eric's suggestion, we can just disable blending. This replaces
> > the previous "Filter out alpha formats for primary plane" patch.
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index fae37b1cd691..1c1dd11d489a 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -961,7 +961,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 (is_alpha_support(fb->format->format)) {
> > +   /*
> > +* 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 (is_alpha_support(fb->format->format) && 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

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

[Sean Paul]

On Mon, Apr 16, 2018 at 03:22:15PM -0700, Kristian H. Kristensen wrote:
> 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.
> 
> Signed-off-by: Kristian H. Kristensen 
> Cc: Sandy Huang 
> Cc: Sean Paul 

Sandy, when you push this, could you please remove the CHROMIUM: prefix from the
subject?

Reviewed-by: Sean Paul 


> ---
> As per Eric's suggestion, we can just disable blending. This replaces
> the previous "Filter out alpha formats for primary plane" patch.
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fae37b1cd691..1c1dd11d489a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -961,7 +961,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 (is_alpha_support(fb->format->format)) {
> + /*
> +  * 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 (is_alpha_support(fb->format->format) && 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[kbuild test robot]

Hi Kristian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16]
[cannot apply to rockchip/for-next drm-exynos/exynos-drm/for-next v4.17-rc1 
next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kristian-H-Kristensen/CHROMIUM-drm-rockchip-Disable-blending-for-win0/20180417-095916
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/rockchip/rockchip_drm_vop.c: In function 
'vop_plane_atomic_update':
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:800:46: error: 'win_index' 
>> undeclared (first use in this function); did you mean 'uuid_index'?
 if (is_alpha_support(fb->format->format) && win_index > 0) {
 ^
 uuid_index
   drivers/gpu/drm/rockchip/rockchip_drm_vop.c:800:46: note: each undeclared 
identifier is reported only once for each function it appears in

vim +800 drivers/gpu/drm/rockchip/rockchip_drm_vop.c

   703  
   704  static void vop_plane_atomic_update(struct drm_plane *plane,
   705  struct drm_plane_state *old_state)
   706  {
   707  struct drm_plane_state *state = plane->state;
   708  struct drm_crtc *crtc = state->crtc;
   709  struct vop_win *vop_win = to_vop_win(plane);
   710  const struct vop_win_data *win = vop_win->data;
   711  struct vop *vop = to_vop(state->crtc);
   712  struct drm_framebuffer *fb = state->fb;
   713  unsigned int actual_w, actual_h;
   714  unsigned int dsp_stx, dsp_sty;
   715  uint32_t act_info, dsp_info, dsp_st;
   716  struct drm_rect *src = >src;
   717  struct drm_rect *dest = >dst;
   718  struct drm_gem_object *obj, *uv_obj;
   719  struct rockchip_gem_object *rk_obj, *rk_uv_obj;
   720  unsigned long offset;
   721  dma_addr_t dma_addr;
   722  uint32_t val;
   723  bool rb_swap;
   724  int format;
   725  
   726  /*
   727   * can't update plane when vop is disabled.
   728   */
   729  if (WARN_ON(!crtc))
   730  return;
   731  
   732  if (WARN_ON(!vop->is_enabled))
   733  return;
   734  
   735  if (!state->visible) {
   736  vop_plane_atomic_disable(plane, old_state);
   737  return;
   738  }
   739  
   740  obj = rockchip_fb_get_gem_obj(fb, 0);
   741  rk_obj = to_rockchip_obj(obj);
   742  
   743  actual_w = drm_rect_width(src) >> 16;
   744  actual_h = drm_rect_height(src) >> 16;
   745  act_info = (actual_h - 1) << 16 | ((actual_w - 1) & 0x);
   746  
   747  dsp_info = (drm_rect_height(dest) - 1) << 16;
   748  dsp_info |= (drm_rect_width(dest) - 1) & 0x;
   749  
   750  dsp_stx = dest->x1 + crtc->mode.htotal - crtc->mode.hsync_start;
   751  dsp_sty = dest->y1 + crtc->mode.vtotal - crtc->mode.vsync_start;
   752  dsp_st = dsp_sty << 16 | (dsp_stx & 0x);
   753  
   754  offset = (src->x1 >> 16) * fb->format->cpp[0];
   755  offset += (src->y1 >> 16) * fb->pitches[0];
   756  dma_addr = rk_obj->dma_addr + offset + fb->offsets[0];
   757  
   758  format = vop_convert_format(fb->format->format);
   759  
   760  spin_lock(>reg_lock);
   761  
   762  VOP_WIN_SET(vop, win, format, format);
   763  VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 
4));
   764  VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
   765  if (is_yuv_support(fb->format->format)) {
   766  int hsub = 
drm_format_horz_chroma_subsampling(fb->format->format);
   767  int vsub = 
drm_format_vert_chroma_subsampling(fb->format->format);
   768  int bpp = fb->format->cpp[1];
   769  
   770  uv_obj = rockchip_fb_get_gem_obj(fb, 1);
   771  rk_uv_obj = to_rockchip_obj(uv_obj);
   772  
   773  offset = (src->x1 >> 16) * bpp / hsub;
   774  offset += (src->y1 >> 16) * fb->pitches[1] / vsub;
   775  
   776  dma_addr = rk_uv_obj->dma_addr + offset + 
fb->offsets[1];
   777  VOP_WIN_SET(vop, win, uv_vir, 
DIV_ROUND_UP(fb->pitches[1], 4));
   778  VOP_WIN_SET(vop, win, uv_mst, dma_addr);
   779  }
   780  
   781  if (win->phy->scl)
   782  

[PATCH] CHROMIUM: drm/rockchip: Disable blending for win0

[Kristian H. Kristensen]

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.

Signed-off-by: Kristian H. Kristensen 
Cc: Sandy Huang 
Cc: Sean Paul 
---
As per Eric's suggestion, we can just disable blending. This replaces
the previous "Filter out alpha formats for primary plane" patch.

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fae37b1cd691..1c1dd11d489a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -961,7 +961,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 (is_alpha_support(fb->format->format)) {
+   /*
+* 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 (is_alpha_support(fb->format->format) && 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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel