| Message ID | 0476e28d70b47ec599fb65d1cfa457276629de27.1513783738.git.digetx@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Some corrections and improvement for Tegra DRM | expand |
On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: > Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke > DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if > requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with > both modesetting and opentegra drivers. On older Tegra's each plane has > a blending configuration which should be used to enable / disable alpha > blending and right now the blending configs are hardcoded to disabled > alpha blending. In order to support alpha formats properly, planes > blending configuration must be adjusted, until then the alpha formats > are equal to non-alpha. > > Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- > drivers/gpu/drm/tegra/dc.h | 1 + > drivers/gpu/drm/tegra/fb.c | 13 ------------- > drivers/gpu/drm/tegra/hub.c | 3 ++- > drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- > drivers/gpu/drm/tegra/plane.h | 2 +- > 6 files changed, 39 insertions(+), 31 deletions(-) This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats. Any comments? Thierry --- >8 --- From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 From: Thierry Reding <treding@nvidia.com> Date: Wed, 20 Dec 2017 09:39:14 +0100 Subject: [PATCH] drm/tegra: dc: Implement legacy blending This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used. Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported. Reported-by: Dmitry Osipenko <digetx@gmail.com> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- drivers/gpu/drm/tegra/dc.h | 13 ++++++++ drivers/gpu/drm/tegra/fb.c | 12 ------- drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ drivers/gpu/drm/tegra/plane.h | 3 ++ 5 files changed, 116 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index bc65c314e00f..07c687d7f615 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) return dfixed_frac(inf); } -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) +static void +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, + const struct tegra_dc_window *window) { + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | + BLEND_COLOR_KEY_NONE; + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | + BLEND_COLOR_KEY_NONE; + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); + + /* enable alpha blending if window->alpha */ + if (window->alpha) { + background |= BLEND_CONTROL_DEPENDENT; + foreground |= BLEND_CONTROL_ALPHA; + } + /* * Disable blending and assume Window A is the bottom-most window, * Window C is the top-most window and Window B is in the middle. */ - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_NOKEY); - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_1WIN); + tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY); + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN); switch (plane->index) { case 0: - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_X); - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y); - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY); + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_X); + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y); + tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY); break; case 1: - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X); - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y); - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY); + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X); + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y); + tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY); break; case 2: - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X); - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_Y); - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_3WIN_XY); + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X); + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_Y); + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_3WIN_XY); break; } } @@ -359,7 +373,7 @@ static void tegra_dc_setup_window(struct tegra_plane *plane, if (dc->soc->supports_blending) { tegra_plane_setup_blending(plane, window); } else { - tegra_plane_setup_blending_legacy(plane); + tegra_plane_setup_blending_legacy(plane, window); } } @@ -370,6 +384,11 @@ static const u32 tegra20_primary_formats[] = { DRM_FORMAT_RGBA5551, DRM_FORMAT_ABGR8888, DRM_FORMAT_ARGB8888, + /* non-native formats */ + DRM_FORMAT_XRGB1555, + DRM_FORMAT_RGBX5551, + DRM_FORMAT_XBGR8888, + DRM_FORMAT_XRGB8888, }; static const u32 tegra114_primary_formats[] = { @@ -426,18 +445,37 @@ static int tegra_plane_atomic_check(struct drm_plane *plane, struct tegra_bo_tiling *tiling = &plane_state->tiling; struct tegra_plane *tegra = to_tegra_plane(plane); struct tegra_dc *dc = to_tegra_dc(state->crtc); + unsigned int format; int err; /* no need for further checks if the plane is being disabled */ if (!state->crtc) return 0; - err = tegra_plane_format(state->fb->format->format, - &plane_state->format, + err = tegra_plane_format(state->fb->format->format, &format, &plane_state->swap); if (err < 0) return err; + /* + * Tegra20 and Tegra30 are special cases here because they support + * only variants of specific formats with an alpha component, but not + * the corresponding opaque formats. However, the opaque formats can + * be emulated by disabling alpha blending for the plane. + */ + if (tegra_plane_format_is_opaque(format) && + !dc->soc->supports_blending) { + err = tegra_plane_format_get_alpha(format, &format); + if (err < 0) + return err; + + plane_state->alpha = false; + } else { + plane_state->alpha = true; + } + + plane_state->format = format; + err = tegra_fb_get_tiling(state->fb, tiling); if (err < 0) return err; @@ -514,6 +552,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane, window.zpos = plane->state->normalized_zpos; window.tiling = state->tiling; window.format = state->format; + window.alpha = state->alpha; window.swap = state->swap; for (i = 0; i < fb->format->num_planes; i++) { @@ -768,6 +807,11 @@ static const u32 tegra20_overlay_formats[] = { DRM_FORMAT_RGBA5551, DRM_FORMAT_ABGR8888, DRM_FORMAT_ARGB8888, + /* non-native formats */ + DRM_FORMAT_XRGB1555, + DRM_FORMAT_RGBX5551, + DRM_FORMAT_XBGR8888, + DRM_FORMAT_XRGB8888, /* planar formats */ DRM_FORMAT_UYVY, DRM_FORMAT_YUYV, diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h index 0c887b53cb10..03b04a7e2814 100644 --- a/drivers/gpu/drm/tegra/dc.h +++ b/drivers/gpu/drm/tegra/dc.h @@ -151,6 +151,7 @@ struct tegra_dc_window { struct tegra_bo_tiling tiling; u32 format; + bool alpha; u32 swap; }; @@ -667,8 +668,20 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc); #define SCALER_COEFF_DATA(x) (((x) & 0x3ff) << 0) #define DC_WIN_BLEND_NOKEY 0x70f +#define BLEND_WEIGHT1(x) (((x) & 0xff) << 16) +#define BLEND_WEIGHT0(x) (((x) & 0xff) << 8) + #define DC_WIN_BLEND_1WIN 0x710 +#define BLEND_CONTROL_FIX (0 << 2) +#define BLEND_CONTROL_ALPHA (1 << 2) +#define BLEND_COLOR_KEY_NONE (0 << 0) +#define BLEND_COLOR_KEY_0 (1 << 0) +#define BLEND_COLOR_KEY_1 (2 << 0) +#define BLEND_COLOR_KEY_BOTH (3 << 0) + #define DC_WIN_BLEND_2WIN_X 0x711 +#define BLEND_CONTROL_DEPENDENT (2 << 2) + #define DC_WIN_BLEND_2WIN_Y 0x712 #define DC_WIN_BLEND_3WIN_XY 0x713 diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index 5f6289c4e56a..001cb77e2f59 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -254,18 +254,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper, cmd.pitches[0] = round_up(sizes->surface_width * bytes_per_pixel, tegra->pitch_align); - /* - * Early generations of Tegra (Tegra20 and Tegra30) do not support any - * of the X* or *X formats, only their A* or *A equivalents. Force the - * legacy framebuffer format to include an alpha component so that the - * framebuffer emulation can be supported on all generations. - */ - if (sizes->surface_bpp == 32 && sizes->surface_depth == 24) - sizes->surface_depth = 32; - - if (sizes->surface_bpp == 16 && sizes->surface_depth == 15) - sizes->surface_depth = 16; - cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index 215051579b3a..dfe991e3d48d 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -51,6 +51,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane) __drm_atomic_helper_plane_duplicate_state(plane, ©->base); copy->tiling = state->tiling; copy->format = state->format; + copy->alpha = state->alpha; copy->swap = state->swap; return ©->base; @@ -256,6 +257,46 @@ bool tegra_plane_format_is_yuv(unsigned int format, bool *planar) return false; } +/* + * This is applicable to Tegra20 and Tegra30 only where the opaque formats can + * be emulated using the alpha formats and blending disabled. + */ +bool tegra_plane_format_is_opaque(unsigned int format) +{ + switch (format) { + case WIN_COLOR_DEPTH_B5G5R5X1: + case WIN_COLOR_DEPTH_X1B5G5R5: + case WIN_COLOR_DEPTH_R8G8B8X8: + case WIN_COLOR_DEPTH_B8G8R8X8: + return true; + } + + return false; +} + +int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha) +{ + switch (opaque) { + case WIN_COLOR_DEPTH_B5G5R5X1: + *alpha = WIN_COLOR_DEPTH_B5G5R5A; + return 0; + + case WIN_COLOR_DEPTH_X1B5G5R5: + *alpha = WIN_COLOR_DEPTH_AB5G5R5; + return 0; + + case WIN_COLOR_DEPTH_R8G8B8X8: + *alpha = WIN_COLOR_DEPTH_R8G8B8A8; + return 0; + + case WIN_COLOR_DEPTH_B8G8R8X8: + *alpha = WIN_COLOR_DEPTH_B8G8R8A8; + return 0; + } + + return -EINVAL; +} + void tegra_plane_get(struct tegra_plane *plane) { mutex_lock(&plane->lock); diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h index 429a086ffe88..06d8b0005fe7 100644 --- a/drivers/gpu/drm/tegra/plane.h +++ b/drivers/gpu/drm/tegra/plane.h @@ -48,6 +48,7 @@ struct tegra_plane_state { struct tegra_bo_tiling tiling; u32 format; + bool alpha; u32 swap; }; @@ -69,6 +70,8 @@ int tegra_plane_state_add(struct tegra_plane *plane, int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap); bool tegra_plane_format_is_yuv(unsigned int format, bool *planar); +bool tegra_plane_format_is_opaque(unsigned int format); +int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha); void tegra_plane_get(struct tegra_plane *plane); void tegra_plane_put(struct tegra_plane *plane);
On 20.12.2017 21:01, Thierry Reding wrote: > On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: >> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke >> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if >> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with >> both modesetting and opentegra drivers. On older Tegra's each plane has >> a blending configuration which should be used to enable / disable alpha >> blending and right now the blending configs are hardcoded to disabled >> alpha blending. In order to support alpha formats properly, planes >> blending configuration must be adjusted, until then the alpha formats >> are equal to non-alpha. >> >> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- >> drivers/gpu/drm/tegra/dc.h | 1 + >> drivers/gpu/drm/tegra/fb.c | 13 ------------- >> drivers/gpu/drm/tegra/hub.c | 3 ++- >> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- >> drivers/gpu/drm/tegra/plane.h | 2 +- >> 6 files changed, 39 insertions(+), 31 deletions(-) > > This kept bugging me, so I spent some time looking at the blending > programming. I came up with the attached patch which seems to work > for all scenarios and is fairly similar to your patch. It has the > added benefit that we can keep support for more formats. > > Any comments? > > Thierry > --- >8 --- > From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 > From: Thierry Reding <treding@nvidia.com> > Date: Wed, 20 Dec 2017 09:39:14 +0100 > Subject: [PATCH] drm/tegra: dc: Implement legacy blending > > This implements alpha blending on legacy display controllers (Tegra20, > Tegra30 and Tegra114). While it's theoretically possible to support the > zpos property to enable userspace to specify the Z-order of each plane > individually, this is not currently supported and the same fixed Z- > order as previously defined is used. Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos. > Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since > the opaque formats are now supported. > > Reported-by: Dmitry Osipenko <digetx@gmail.com> > Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- > drivers/gpu/drm/tegra/dc.h | 13 ++++++++ > drivers/gpu/drm/tegra/fb.c | 12 ------- > drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ > drivers/gpu/drm/tegra/plane.h | 3 ++ > 5 files changed, 116 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > index bc65c314e00f..07c687d7f615 100644 > --- a/drivers/gpu/drm/tegra/dc.c > +++ b/drivers/gpu/drm/tegra/dc.c > @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) > return dfixed_frac(inf); > } > > -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) > +static void > +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, > + const struct tegra_dc_window *window) > { > + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | > + BLEND_COLOR_KEY_NONE; > + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | > + BLEND_COLOR_KEY_NONE; > + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); > + > + /* enable alpha blending if window->alpha */ > + if (window->alpha) { > + background |= BLEND_CONTROL_DEPENDENT; > + foreground |= BLEND_CONTROL_ALPHA; > + } I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel. If the above is correct, then I'm suggesting to not expose alpha formats, we should properly test all combinations of blending of all the windows. In one case you could apply my patch for now and then me/you/we could work on a proper legacy blending implementation based on your patch. In the other case I could take your patch into v4 (cursor patch would have to be rabased in that case) and we will correct blending sometime later. I don't mind either case, up to you to decide. Is there any ready-made testsuite for the DRM planes blending? Or you have made a test by yourself? In the latter case, could you please share the code so that I could test it too without burden of writing testcases from scratch? > + > /* > * Disable blending and assume Window A is the bottom-most window, > * Window C is the top-most window and Window B is in the middle. > */ > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_NOKEY); > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_1WIN); > + tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY); > + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN); > > switch (plane->index) { > case 0: > - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_X); > - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y); > - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY); > + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_X); > + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y); > + tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY); > break; > > case 1: > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X); > - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y); > - tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY); > + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X); > + tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y); > + tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY); > break; > > case 2: > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X); > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_Y); > - tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_3WIN_XY); > + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X); > + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_Y); > + tegra_plane_writel(plane, foreground, DC_WIN_BLEND_3WIN_XY); > break; > } > } > @@ -359,7 +373,7 @@ static void tegra_dc_setup_window(struct tegra_plane *plane, > if (dc->soc->supports_blending) { > tegra_plane_setup_blending(plane, window); > } else { > - tegra_plane_setup_blending_legacy(plane); > + tegra_plane_setup_blending_legacy(plane, window); > } > } > > @@ -370,6 +384,11 @@ static const u32 tegra20_primary_formats[] = { > DRM_FORMAT_RGBA5551, > DRM_FORMAT_ABGR8888, > DRM_FORMAT_ARGB8888, > + /* non-native formats */ > + DRM_FORMAT_XRGB1555, > + DRM_FORMAT_RGBX5551, > + DRM_FORMAT_XBGR8888, > + DRM_FORMAT_XRGB8888, > }; > > static const u32 tegra114_primary_formats[] = { > @@ -426,18 +445,37 @@ static int tegra_plane_atomic_check(struct drm_plane *plane, > struct tegra_bo_tiling *tiling = &plane_state->tiling; > struct tegra_plane *tegra = to_tegra_plane(plane); > struct tegra_dc *dc = to_tegra_dc(state->crtc); > + unsigned int format; > int err; > > /* no need for further checks if the plane is being disabled */ > if (!state->crtc) > return 0; > > - err = tegra_plane_format(state->fb->format->format, > - &plane_state->format, > + err = tegra_plane_format(state->fb->format->format, &format, > &plane_state->swap); > if (err < 0) > return err; > > + /* > + * Tegra20 and Tegra30 are special cases here because they support > + * only variants of specific formats with an alpha component, but not > + * the corresponding opaque formats. However, the opaque formats can > + * be emulated by disabling alpha blending for the plane. > + */ > + if (tegra_plane_format_is_opaque(format) && > + !dc->soc->supports_blending) { > + err = tegra_plane_format_get_alpha(format, &format); > + if (err < 0) > + return err; > + > + plane_state->alpha = false; > + } else { > + plane_state->alpha = true; > + } > + > + plane_state->format = format; > + > err = tegra_fb_get_tiling(state->fb, tiling); > if (err < 0) > return err; > @@ -514,6 +552,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane, > window.zpos = plane->state->normalized_zpos; > window.tiling = state->tiling; > window.format = state->format; > + window.alpha = state->alpha; > window.swap = state->swap; > > for (i = 0; i < fb->format->num_planes; i++) { > @@ -768,6 +807,11 @@ static const u32 tegra20_overlay_formats[] = { > DRM_FORMAT_RGBA5551, > DRM_FORMAT_ABGR8888, > DRM_FORMAT_ARGB8888, > + /* non-native formats */ > + DRM_FORMAT_XRGB1555, > + DRM_FORMAT_RGBX5551, > + DRM_FORMAT_XBGR8888, > + DRM_FORMAT_XRGB8888, > /* planar formats */ > DRM_FORMAT_UYVY, > DRM_FORMAT_YUYV, > diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h > index 0c887b53cb10..03b04a7e2814 100644 > --- a/drivers/gpu/drm/tegra/dc.h > +++ b/drivers/gpu/drm/tegra/dc.h > @@ -151,6 +151,7 @@ struct tegra_dc_window { > > struct tegra_bo_tiling tiling; > u32 format; > + bool alpha; > u32 swap; > }; > > @@ -667,8 +668,20 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc); > #define SCALER_COEFF_DATA(x) (((x) & 0x3ff) << 0) > > #define DC_WIN_BLEND_NOKEY 0x70f > +#define BLEND_WEIGHT1(x) (((x) & 0xff) << 16) > +#define BLEND_WEIGHT0(x) (((x) & 0xff) << 8) > + > #define DC_WIN_BLEND_1WIN 0x710 > +#define BLEND_CONTROL_FIX (0 << 2) > +#define BLEND_CONTROL_ALPHA (1 << 2) > +#define BLEND_COLOR_KEY_NONE (0 << 0) > +#define BLEND_COLOR_KEY_0 (1 << 0) > +#define BLEND_COLOR_KEY_1 (2 << 0) > +#define BLEND_COLOR_KEY_BOTH (3 << 0) > + > #define DC_WIN_BLEND_2WIN_X 0x711 > +#define BLEND_CONTROL_DEPENDENT (2 << 2) > + > #define DC_WIN_BLEND_2WIN_Y 0x712 > #define DC_WIN_BLEND_3WIN_XY 0x713 > > diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c > index 5f6289c4e56a..001cb77e2f59 100644 > --- a/drivers/gpu/drm/tegra/fb.c > +++ b/drivers/gpu/drm/tegra/fb.c > @@ -254,18 +254,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper, > cmd.pitches[0] = round_up(sizes->surface_width * bytes_per_pixel, > tegra->pitch_align); > > - /* > - * Early generations of Tegra (Tegra20 and Tegra30) do not support any > - * of the X* or *X formats, only their A* or *A equivalents. Force the > - * legacy framebuffer format to include an alpha component so that the > - * framebuffer emulation can be supported on all generations. > - */ > - if (sizes->surface_bpp == 32 && sizes->surface_depth == 24) > - sizes->surface_depth = 32; > - > - if (sizes->surface_bpp == 16 && sizes->surface_depth == 15) > - sizes->surface_depth = 16; > - > cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, > sizes->surface_depth); > > diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c > index 215051579b3a..dfe991e3d48d 100644 > --- a/drivers/gpu/drm/tegra/plane.c > +++ b/drivers/gpu/drm/tegra/plane.c > @@ -51,6 +51,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane) > __drm_atomic_helper_plane_duplicate_state(plane, ©->base); > copy->tiling = state->tiling; > copy->format = state->format; > + copy->alpha = state->alpha; > copy->swap = state->swap; > > return ©->base; > @@ -256,6 +257,46 @@ bool tegra_plane_format_is_yuv(unsigned int format, bool *planar) > return false; > } > > +/* > + * This is applicable to Tegra20 and Tegra30 only where the opaque formats can > + * be emulated using the alpha formats and blending disabled. > + */ > +bool tegra_plane_format_is_opaque(unsigned int format) > +{ > + switch (format) { > + case WIN_COLOR_DEPTH_B5G5R5X1: > + case WIN_COLOR_DEPTH_X1B5G5R5: > + case WIN_COLOR_DEPTH_R8G8B8X8: > + case WIN_COLOR_DEPTH_B8G8R8X8: > + return true; > + } > + > + return false; > +} > + > +int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha) > +{ > + switch (opaque) { > + case WIN_COLOR_DEPTH_B5G5R5X1: > + *alpha = WIN_COLOR_DEPTH_B5G5R5A; > + return 0; > + > + case WIN_COLOR_DEPTH_X1B5G5R5: > + *alpha = WIN_COLOR_DEPTH_AB5G5R5; > + return 0; > + > + case WIN_COLOR_DEPTH_R8G8B8X8: > + *alpha = WIN_COLOR_DEPTH_R8G8B8A8; > + return 0; > + > + case WIN_COLOR_DEPTH_B8G8R8X8: > + *alpha = WIN_COLOR_DEPTH_B8G8R8A8; > + return 0; > + } > + > + return -EINVAL; > +} > + > void tegra_plane_get(struct tegra_plane *plane) > { > mutex_lock(&plane->lock); > diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h > index 429a086ffe88..06d8b0005fe7 100644 > --- a/drivers/gpu/drm/tegra/plane.h > +++ b/drivers/gpu/drm/tegra/plane.h > @@ -48,6 +48,7 @@ struct tegra_plane_state { > > struct tegra_bo_tiling tiling; > u32 format; > + bool alpha; > u32 swap; > }; > > @@ -69,6 +70,8 @@ int tegra_plane_state_add(struct tegra_plane *plane, > > int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap); > bool tegra_plane_format_is_yuv(unsigned int format, bool *planar); > +bool tegra_plane_format_is_opaque(unsigned int format); > +int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha); > > void tegra_plane_get(struct tegra_plane *plane); > void tegra_plane_put(struct tegra_plane *plane); > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: > On 20.12.2017 21:01, Thierry Reding wrote: > > On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: > >> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke > >> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if > >> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with > >> both modesetting and opentegra drivers. On older Tegra's each plane has > >> a blending configuration which should be used to enable / disable alpha > >> blending and right now the blending configs are hardcoded to disabled > >> alpha blending. In order to support alpha formats properly, planes > >> blending configuration must be adjusted, until then the alpha formats > >> are equal to non-alpha. > >> > >> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- > >> drivers/gpu/drm/tegra/dc.h | 1 + > >> drivers/gpu/drm/tegra/fb.c | 13 ------------- > >> drivers/gpu/drm/tegra/hub.c | 3 ++- > >> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- > >> drivers/gpu/drm/tegra/plane.h | 2 +- > >> 6 files changed, 39 insertions(+), 31 deletions(-) > > > > This kept bugging me, so I spent some time looking at the blending > > programming. I came up with the attached patch which seems to work > > for all scenarios and is fairly similar to your patch. It has the > > added benefit that we can keep support for more formats. > > > > Any comments? > > > > Thierry > > --- >8 --- > > From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 > > From: Thierry Reding <treding@nvidia.com> > > Date: Wed, 20 Dec 2017 09:39:14 +0100 > > Subject: [PATCH] drm/tegra: dc: Implement legacy blending > > > > This implements alpha blending on legacy display controllers (Tegra20, > > Tegra30 and Tegra114). While it's theoretically possible to support the > > zpos property to enable userspace to specify the Z-order of each plane > > individually, this is not currently supported and the same fixed Z- > > order as previously defined is used. > > Perhaps one variant of implementing zpos could be by making overlays 'virtual', > so each virtual overlay will be backed by the real HW plane and we could swap > the HW planes of the virtual overlays, emulating zpos. > > > Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since > > the opaque formats are now supported. > > > > Reported-by: Dmitry Osipenko <digetx@gmail.com> > > Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- > > drivers/gpu/drm/tegra/dc.h | 13 ++++++++ > > drivers/gpu/drm/tegra/fb.c | 12 ------- > > drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ > > drivers/gpu/drm/tegra/plane.h | 3 ++ > > 5 files changed, 116 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > > index bc65c314e00f..07c687d7f615 100644 > > --- a/drivers/gpu/drm/tegra/dc.c > > +++ b/drivers/gpu/drm/tegra/dc.c > > @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) > > return dfixed_frac(inf); > > } > > > > -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) > > +static void > > +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, > > + const struct tegra_dc_window *window) > > { > > + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | > > + BLEND_COLOR_KEY_NONE; > > + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | > > + BLEND_COLOR_KEY_NONE; > > + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); > > + > > + /* enable alpha blending if window->alpha */ > > + if (window->alpha) { > > + background |= BLEND_CONTROL_DEPENDENT; > > + foreground |= BLEND_CONTROL_ALPHA; > > + } > > I think dependent weight means that window doesn't have alpha transparency. So > we should set the dependent_weight mode for opaque formats and alpha_weight for > formats with alpha channel. According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below). > If the above is correct, then I'm suggesting to not expose alpha formats, we > should properly test all combinations of blending of all the windows. In one > case you could apply my patch for now and then me/you/we could work on a proper > legacy blending implementation based on your patch. In the other case I could > take your patch into v4 (cursor patch would have to be rabased in that case) and > we will correct blending sometime later. I don't mind either case, up to you to > decide. The problem with not exposing alpha formats is that we either have to omit them everywhere or implement complicated workarounds that make the old hardware special. That's still true to some degree in this patch, but I think it's fairly cleanly split out. > Is there any ready-made testsuite for the DRM planes blending? Or you have made > a test by yourself? In the latter case, could you please share the code so that > I could test it too without burden of writing testcases from scratch? I've implemented a test for this in my fork of libdrm: https://cgit.freedesktop.org/~tagr/drm/ The kms-planes-blending test will fill three framebuffers with the primary colors at 0.5 alpha and presents them in three planes that overlay at the screen center. I tested it on TrimSlice and the result looks correct. Thierry
On 20.12.2017 23:16, Thierry Reding wrote: > On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: >> On 20.12.2017 21:01, Thierry Reding wrote: >>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: >>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke >>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if >>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with >>>> both modesetting and opentegra drivers. On older Tegra's each plane has >>>> a blending configuration which should be used to enable / disable alpha >>>> blending and right now the blending configs are hardcoded to disabled >>>> alpha blending. In order to support alpha formats properly, planes >>>> blending configuration must be adjusted, until then the alpha formats >>>> are equal to non-alpha. >>>> >>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>> --- >>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- >>>> drivers/gpu/drm/tegra/dc.h | 1 + >>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- >>>> drivers/gpu/drm/tegra/hub.c | 3 ++- >>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- >>>> drivers/gpu/drm/tegra/plane.h | 2 +- >>>> 6 files changed, 39 insertions(+), 31 deletions(-) >>> >>> This kept bugging me, so I spent some time looking at the blending >>> programming. I came up with the attached patch which seems to work >>> for all scenarios and is fairly similar to your patch. It has the >>> added benefit that we can keep support for more formats. >>> >>> Any comments? >>> >>> Thierry >>> --- >8 --- >>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 >>> From: Thierry Reding <treding@nvidia.com> >>> Date: Wed, 20 Dec 2017 09:39:14 +0100 >>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending >>> >>> This implements alpha blending on legacy display controllers (Tegra20, >>> Tegra30 and Tegra114). While it's theoretically possible to support the >>> zpos property to enable userspace to specify the Z-order of each plane >>> individually, this is not currently supported and the same fixed Z- >>> order as previously defined is used. >> >> Perhaps one variant of implementing zpos could be by making overlays 'virtual', >> so each virtual overlay will be backed by the real HW plane and we could swap >> the HW planes of the virtual overlays, emulating zpos. >> >>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since >>> the opaque formats are now supported. >>> >>> Reported-by: Dmitry Osipenko <digetx@gmail.com> >>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>> --- >>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- >>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ >>> drivers/gpu/drm/tegra/fb.c | 12 ------- >>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ >>> drivers/gpu/drm/tegra/plane.h | 3 ++ >>> 5 files changed, 116 insertions(+), 27 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c >>> index bc65c314e00f..07c687d7f615 100644 >>> --- a/drivers/gpu/drm/tegra/dc.c >>> +++ b/drivers/gpu/drm/tegra/dc.c >>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) >>> return dfixed_frac(inf); >>> } >>> >>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) >>> +static void >>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, >>> + const struct tegra_dc_window *window) >>> { >>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | >>> + BLEND_COLOR_KEY_NONE; >>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | >>> + BLEND_COLOR_KEY_NONE; >>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); >>> + >>> + /* enable alpha blending if window->alpha */ >>> + if (window->alpha) { >>> + background |= BLEND_CONTROL_DEPENDENT; >>> + foreground |= BLEND_CONTROL_ALPHA; >>> + } >> >> I think dependent weight means that window doesn't have alpha transparency. So >> we should set the dependent_weight mode for opaque formats and alpha_weight for >> formats with alpha channel. > > According to the TRM, dependent weight means that its weight will be 1 - > the sum of the other overlapping windows. And it certainly seems to work > that way in my tests (see below). Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque. >> If the above is correct, then I'm suggesting to not expose alpha formats, we >> should properly test all combinations of blending of all the windows. In one >> case you could apply my patch for now and then me/you/we could work on a proper >> legacy blending implementation based on your patch. In the other case I could >> take your patch into v4 (cursor patch would have to be rabased in that case) and >> we will correct blending sometime later. I don't mind either case, up to you to >> decide. > > The problem with not exposing alpha formats is that we either have to > omit them everywhere or implement complicated workarounds that make the > old hardware special. That's still true to some degree in this patch, > but I think it's fairly cleanly split out. > >> Is there any ready-made testsuite for the DRM planes blending? Or you have made >> a test by yourself? In the latter case, could you please share the code so that >> I could test it too without burden of writing testcases from scratch? > > I've implemented a test for this in my fork of libdrm: > > https://cgit.freedesktop.org/~tagr/drm/ > > The kms-planes-blending test will fill three framebuffers with the > primary colors at 0.5 alpha and presents them in three planes that > overlay at the screen center. > > I tested it on TrimSlice and the result looks correct. But maybe I'm missing something. Thanks for sharing the test, I'll try it out. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote: > On 20.12.2017 23:16, Thierry Reding wrote: > > On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: > >> On 20.12.2017 21:01, Thierry Reding wrote: > >>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: > >>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke > >>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if > >>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with > >>>> both modesetting and opentegra drivers. On older Tegra's each plane has > >>>> a blending configuration which should be used to enable / disable alpha > >>>> blending and right now the blending configs are hardcoded to disabled > >>>> alpha blending. In order to support alpha formats properly, planes > >>>> blending configuration must be adjusted, until then the alpha formats > >>>> are equal to non-alpha. > >>>> > >>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >>>> --- > >>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- > >>>> drivers/gpu/drm/tegra/dc.h | 1 + > >>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- > >>>> drivers/gpu/drm/tegra/hub.c | 3 ++- > >>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- > >>>> drivers/gpu/drm/tegra/plane.h | 2 +- > >>>> 6 files changed, 39 insertions(+), 31 deletions(-) > >>> > >>> This kept bugging me, so I spent some time looking at the blending > >>> programming. I came up with the attached patch which seems to work > >>> for all scenarios and is fairly similar to your patch. It has the > >>> added benefit that we can keep support for more formats. > >>> > >>> Any comments? > >>> > >>> Thierry > >>> --- >8 --- > >>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 > >>> From: Thierry Reding <treding@nvidia.com> > >>> Date: Wed, 20 Dec 2017 09:39:14 +0100 > >>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending > >>> > >>> This implements alpha blending on legacy display controllers (Tegra20, > >>> Tegra30 and Tegra114). While it's theoretically possible to support the > >>> zpos property to enable userspace to specify the Z-order of each plane > >>> individually, this is not currently supported and the same fixed Z- > >>> order as previously defined is used. > >> > >> Perhaps one variant of implementing zpos could be by making overlays 'virtual', > >> so each virtual overlay will be backed by the real HW plane and we could swap > >> the HW planes of the virtual overlays, emulating zpos. > >> > >>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since > >>> the opaque formats are now supported. > >>> > >>> Reported-by: Dmitry Osipenko <digetx@gmail.com> > >>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > >>> Signed-off-by: Thierry Reding <treding@nvidia.com> > >>> --- > >>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- > >>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ > >>> drivers/gpu/drm/tegra/fb.c | 12 ------- > >>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ > >>> drivers/gpu/drm/tegra/plane.h | 3 ++ > >>> 5 files changed, 116 insertions(+), 27 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > >>> index bc65c314e00f..07c687d7f615 100644 > >>> --- a/drivers/gpu/drm/tegra/dc.c > >>> +++ b/drivers/gpu/drm/tegra/dc.c > >>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) > >>> return dfixed_frac(inf); > >>> } > >>> > >>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) > >>> +static void > >>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, > >>> + const struct tegra_dc_window *window) > >>> { > >>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | > >>> + BLEND_COLOR_KEY_NONE; > >>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | > >>> + BLEND_COLOR_KEY_NONE; > >>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); > >>> + > >>> + /* enable alpha blending if window->alpha */ > >>> + if (window->alpha) { > >>> + background |= BLEND_CONTROL_DEPENDENT; > >>> + foreground |= BLEND_CONTROL_ALPHA; > >>> + } > >> > >> I think dependent weight means that window doesn't have alpha transparency. So > >> we should set the dependent_weight mode for opaque formats and alpha_weight for > >> formats with alpha channel. > > > > According to the TRM, dependent weight means that its weight will be 1 - > > the sum of the other overlapping windows. And it certainly seems to work > > that way in my tests (see below). > > Yes, and you are hardcoding the blending modes regardless of the actual plane > format. So even if underlying window has alpha format, it will be treated as it > is opaque. Ah yes, indeed. The patch currently assumes that if the current plane has an alpha component, then all the others will, too. That can probably be done fairly easily by looking at the current atomic state and inspecting the pixel format for each plane on the same CRTC. Let me take a stab at that tomorrow. I'm wondering if we can deal with zpos, too, if we're already going through the trouble of looking at all planes involved. I think we can simply permute the WIN_BLEND register writes depending on the Z-order to implement proper zpos support. Thierry
On 21.12.2017 01:02, Thierry Reding wrote: > On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote: >> On 20.12.2017 23:16, Thierry Reding wrote: >>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: >>>> On 20.12.2017 21:01, Thierry Reding wrote: >>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: >>>>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke >>>>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if >>>>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with >>>>>> both modesetting and opentegra drivers. On older Tegra's each plane has >>>>>> a blending configuration which should be used to enable / disable alpha >>>>>> blending and right now the blending configs are hardcoded to disabled >>>>>> alpha blending. In order to support alpha formats properly, planes >>>>>> blending configuration must be adjusted, until then the alpha formats >>>>>> are equal to non-alpha. >>>>>> >>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>>>> --- >>>>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- >>>>>> drivers/gpu/drm/tegra/dc.h | 1 + >>>>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- >>>>>> drivers/gpu/drm/tegra/hub.c | 3 ++- >>>>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- >>>>>> drivers/gpu/drm/tegra/plane.h | 2 +- >>>>>> 6 files changed, 39 insertions(+), 31 deletions(-) >>>>> >>>>> This kept bugging me, so I spent some time looking at the blending >>>>> programming. I came up with the attached patch which seems to work >>>>> for all scenarios and is fairly similar to your patch. It has the >>>>> added benefit that we can keep support for more formats. >>>>> >>>>> Any comments? >>>>> >>>>> Thierry >>>>> --- >8 --- >>>>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 >>>>> From: Thierry Reding <treding@nvidia.com> >>>>> Date: Wed, 20 Dec 2017 09:39:14 +0100 >>>>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending >>>>> >>>>> This implements alpha blending on legacy display controllers (Tegra20, >>>>> Tegra30 and Tegra114). While it's theoretically possible to support the >>>>> zpos property to enable userspace to specify the Z-order of each plane >>>>> individually, this is not currently supported and the same fixed Z- >>>>> order as previously defined is used. >>>> >>>> Perhaps one variant of implementing zpos could be by making overlays 'virtual', >>>> so each virtual overlay will be backed by the real HW plane and we could swap >>>> the HW planes of the virtual overlays, emulating zpos. >>>> >>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since >>>>> the opaque formats are now supported. >>>>> >>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com> >>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>>> --- >>>>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- >>>>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ >>>>> drivers/gpu/drm/tegra/fb.c | 12 ------- >>>>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ >>>>> drivers/gpu/drm/tegra/plane.h | 3 ++ >>>>> 5 files changed, 116 insertions(+), 27 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c >>>>> index bc65c314e00f..07c687d7f615 100644 >>>>> --- a/drivers/gpu/drm/tegra/dc.c >>>>> +++ b/drivers/gpu/drm/tegra/dc.c >>>>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) >>>>> return dfixed_frac(inf); >>>>> } >>>>> >>>>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) >>>>> +static void >>>>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, >>>>> + const struct tegra_dc_window *window) >>>>> { >>>>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | >>>>> + BLEND_COLOR_KEY_NONE; >>>>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | >>>>> + BLEND_COLOR_KEY_NONE; >>>>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); >>>>> + >>>>> + /* enable alpha blending if window->alpha */ >>>>> + if (window->alpha) { >>>>> + background |= BLEND_CONTROL_DEPENDENT; >>>>> + foreground |= BLEND_CONTROL_ALPHA; >>>>> + } >>>> >>>> I think dependent weight means that window doesn't have alpha transparency. So >>>> we should set the dependent_weight mode for opaque formats and alpha_weight for >>>> formats with alpha channel. >>> >>> According to the TRM, dependent weight means that its weight will be 1 - >>> the sum of the other overlapping windows. And it certainly seems to work >>> that way in my tests (see below). >> >> Yes, and you are hardcoding the blending modes regardless of the actual plane >> format. So even if underlying window has alpha format, it will be treated as it >> is opaque. > > Ah yes, indeed. The patch currently assumes that if the current plane > has an alpha component, then all the others will, too. That can probably > be done fairly easily by looking at the current atomic state and > inspecting the pixel format for each plane on the same CRTC. Let me take > a stab at that tomorrow. Okay, please push patch to the public repo once it is ready and let me know. I'll rebase cursor patch on it. > I'm wondering if we can deal with zpos, too, if we're already going > through the trouble of looking at all planes involved. I think we can > simply permute the WIN_BLEND register writes depending on the Z-order > to implement proper zpos support. I think we will have to permute the whole window state, not only the WIN_BLEND register. Also I'm not sure how to make topmost window opaque and underlying windows transparent, will have to check how blending works. Maybe it not possible at all.. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21.12.2017 01:23, Dmitry Osipenko wrote: > On 21.12.2017 01:02, Thierry Reding wrote: >> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote: >>> On 20.12.2017 23:16, Thierry Reding wrote: >>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: >>>>> On 20.12.2017 21:01, Thierry Reding wrote: >>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: >>>>>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke >>>>>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if >>>>>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with >>>>>>> both modesetting and opentegra drivers. On older Tegra's each plane has >>>>>>> a blending configuration which should be used to enable / disable alpha >>>>>>> blending and right now the blending configs are hardcoded to disabled >>>>>>> alpha blending. In order to support alpha formats properly, planes >>>>>>> blending configuration must be adjusted, until then the alpha formats >>>>>>> are equal to non-alpha. >>>>>>> >>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- >>>>>>> drivers/gpu/drm/tegra/dc.h | 1 + >>>>>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- >>>>>>> drivers/gpu/drm/tegra/hub.c | 3 ++- >>>>>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- >>>>>>> drivers/gpu/drm/tegra/plane.h | 2 +- >>>>>>> 6 files changed, 39 insertions(+), 31 deletions(-) >>>>>> >>>>>> This kept bugging me, so I spent some time looking at the blending >>>>>> programming. I came up with the attached patch which seems to work >>>>>> for all scenarios and is fairly similar to your patch. It has the >>>>>> added benefit that we can keep support for more formats. >>>>>> >>>>>> Any comments? >>>>>> >>>>>> Thierry >>>>>> --- >8 --- >>>>>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 >>>>>> From: Thierry Reding <treding@nvidia.com> >>>>>> Date: Wed, 20 Dec 2017 09:39:14 +0100 >>>>>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending >>>>>> >>>>>> This implements alpha blending on legacy display controllers (Tegra20, >>>>>> Tegra30 and Tegra114). While it's theoretically possible to support the >>>>>> zpos property to enable userspace to specify the Z-order of each plane >>>>>> individually, this is not currently supported and the same fixed Z- >>>>>> order as previously defined is used. >>>>> >>>>> Perhaps one variant of implementing zpos could be by making overlays 'virtual', >>>>> so each virtual overlay will be backed by the real HW plane and we could swap >>>>> the HW planes of the virtual overlays, emulating zpos. >>>>> >>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since >>>>>> the opaque formats are now supported. >>>>>> >>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com> >>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>>>> --- >>>>>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- >>>>>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ >>>>>> drivers/gpu/drm/tegra/fb.c | 12 ------- >>>>>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ >>>>>> drivers/gpu/drm/tegra/plane.h | 3 ++ >>>>>> 5 files changed, 116 insertions(+), 27 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c >>>>>> index bc65c314e00f..07c687d7f615 100644 >>>>>> --- a/drivers/gpu/drm/tegra/dc.c >>>>>> +++ b/drivers/gpu/drm/tegra/dc.c >>>>>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) >>>>>> return dfixed_frac(inf); >>>>>> } >>>>>> >>>>>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) >>>>>> +static void >>>>>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, >>>>>> + const struct tegra_dc_window *window) >>>>>> { >>>>>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | >>>>>> + BLEND_COLOR_KEY_NONE; >>>>>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | >>>>>> + BLEND_COLOR_KEY_NONE; >>>>>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); >>>>>> + >>>>>> + /* enable alpha blending if window->alpha */ >>>>>> + if (window->alpha) { >>>>>> + background |= BLEND_CONTROL_DEPENDENT; >>>>>> + foreground |= BLEND_CONTROL_ALPHA; >>>>>> + } >>>>> >>>>> I think dependent weight means that window doesn't have alpha transparency. So >>>>> we should set the dependent_weight mode for opaque formats and alpha_weight for >>>>> formats with alpha channel. >>>> >>>> According to the TRM, dependent weight means that its weight will be 1 - >>>> the sum of the other overlapping windows. And it certainly seems to work >>>> that way in my tests (see below). >>> >>> Yes, and you are hardcoding the blending modes regardless of the actual plane >>> format. So even if underlying window has alpha format, it will be treated as it >>> is opaque. >> >> Ah yes, indeed. The patch currently assumes that if the current plane >> has an alpha component, then all the others will, too. That can probably >> be done fairly easily by looking at the current atomic state and >> inspecting the pixel format for each plane on the same CRTC. Let me take >> a stab at that tomorrow. > > Okay, please push patch to the public repo once it is ready and let me know. > I'll rebase cursor patch on it. > >> I'm wondering if we can deal with zpos, too, if we're already going >> through the trouble of looking at all planes involved. I think we can >> simply permute the WIN_BLEND register writes depending on the Z-order >> to implement proper zpos support. > I think we will have to permute the whole window state, not only the WIN_BLEND > register. > > Also I'm not sure how to make topmost window opaque and underlying windows > transparent, will have to check how blending works. Maybe it not possible at all.. Although it is simple to implement using the fixed weights for 2WIN cases, but then we will have to enhance the legacy blending code a tad to handle this case properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can still apply my patch that drops alpha formats on T20/30 and we will try to implement alpha + zpos for 4.17. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote: > On 21.12.2017 01:23, Dmitry Osipenko wrote: > > On 21.12.2017 01:02, Thierry Reding wrote: > >> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote: > >>> On 20.12.2017 23:16, Thierry Reding wrote: > >>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: > >>>>> On 20.12.2017 21:01, Thierry Reding wrote: > >>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: > >>>>>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke > >>>>>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if > >>>>>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with > >>>>>>> both modesetting and opentegra drivers. On older Tegra's each plane has > >>>>>>> a blending configuration which should be used to enable / disable alpha > >>>>>>> blending and right now the blending configs are hardcoded to disabled > >>>>>>> alpha blending. In order to support alpha formats properly, planes > >>>>>>> blending configuration must be adjusted, until then the alpha formats > >>>>>>> are equal to non-alpha. > >>>>>>> > >>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > >>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >>>>>>> --- > >>>>>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- > >>>>>>> drivers/gpu/drm/tegra/dc.h | 1 + > >>>>>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- > >>>>>>> drivers/gpu/drm/tegra/hub.c | 3 ++- > >>>>>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- > >>>>>>> drivers/gpu/drm/tegra/plane.h | 2 +- > >>>>>>> 6 files changed, 39 insertions(+), 31 deletions(-) > >>>>>> > >>>>>> This kept bugging me, so I spent some time looking at the blending > >>>>>> programming. I came up with the attached patch which seems to work > >>>>>> for all scenarios and is fairly similar to your patch. It has the > >>>>>> added benefit that we can keep support for more formats. > >>>>>> > >>>>>> Any comments? > >>>>>> > >>>>>> Thierry > >>>>>> --- >8 --- > >>>>>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 > >>>>>> From: Thierry Reding <treding@nvidia.com> > >>>>>> Date: Wed, 20 Dec 2017 09:39:14 +0100 > >>>>>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending > >>>>>> > >>>>>> This implements alpha blending on legacy display controllers (Tegra20, > >>>>>> Tegra30 and Tegra114). While it's theoretically possible to support the > >>>>>> zpos property to enable userspace to specify the Z-order of each plane > >>>>>> individually, this is not currently supported and the same fixed Z- > >>>>>> order as previously defined is used. > >>>>> > >>>>> Perhaps one variant of implementing zpos could be by making overlays 'virtual', > >>>>> so each virtual overlay will be backed by the real HW plane and we could swap > >>>>> the HW planes of the virtual overlays, emulating zpos. > >>>>> > >>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since > >>>>>> the opaque formats are now supported. > >>>>>> > >>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com> > >>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") > >>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com> > >>>>>> --- > >>>>>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- > >>>>>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ > >>>>>> drivers/gpu/drm/tegra/fb.c | 12 ------- > >>>>>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ > >>>>>> drivers/gpu/drm/tegra/plane.h | 3 ++ > >>>>>> 5 files changed, 116 insertions(+), 27 deletions(-) > >>>>>> > >>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > >>>>>> index bc65c314e00f..07c687d7f615 100644 > >>>>>> --- a/drivers/gpu/drm/tegra/dc.c > >>>>>> +++ b/drivers/gpu/drm/tegra/dc.c > >>>>>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) > >>>>>> return dfixed_frac(inf); > >>>>>> } > >>>>>> > >>>>>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) > >>>>>> +static void > >>>>>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, > >>>>>> + const struct tegra_dc_window *window) > >>>>>> { > >>>>>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | > >>>>>> + BLEND_COLOR_KEY_NONE; > >>>>>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | > >>>>>> + BLEND_COLOR_KEY_NONE; > >>>>>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); > >>>>>> + > >>>>>> + /* enable alpha blending if window->alpha */ > >>>>>> + if (window->alpha) { > >>>>>> + background |= BLEND_CONTROL_DEPENDENT; > >>>>>> + foreground |= BLEND_CONTROL_ALPHA; > >>>>>> + } > >>>>> > >>>>> I think dependent weight means that window doesn't have alpha transparency. So > >>>>> we should set the dependent_weight mode for opaque formats and alpha_weight for > >>>>> formats with alpha channel. > >>>> > >>>> According to the TRM, dependent weight means that its weight will be 1 - > >>>> the sum of the other overlapping windows. And it certainly seems to work > >>>> that way in my tests (see below). > >>> > >>> Yes, and you are hardcoding the blending modes regardless of the actual plane > >>> format. So even if underlying window has alpha format, it will be treated as it > >>> is opaque. > >> > >> Ah yes, indeed. The patch currently assumes that if the current plane > >> has an alpha component, then all the others will, too. That can probably > >> be done fairly easily by looking at the current atomic state and > >> inspecting the pixel format for each plane on the same CRTC. Let me take > >> a stab at that tomorrow. > > > > Okay, please push patch to the public repo once it is ready and let me know. > > I'll rebase cursor patch on it. > > > >> I'm wondering if we can deal with zpos, too, if we're already going > >> through the trouble of looking at all planes involved. I think we can > >> simply permute the WIN_BLEND register writes depending on the Z-order > >> to implement proper zpos support. > > I think we will have to permute the whole window state, not only the WIN_BLEND > > register. > > > > Also I'm not sure how to make topmost window opaque and underlying windows > > transparent, will have to check how blending works. Maybe it not possible at all.. > > Although it is simple to implement using the fixed weights for 2WIN cases, but > then we will have to enhance the legacy blending code a tad to handle this case > properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can > still apply my patch that drops alpha formats on T20/30 and we will try to > implement alpha + zpos for 4.17. I think I finally nailed this. Just sent out a v2 and updated my test program to use different formats for each plane. I tested all opaque & alpha combinations and it all looks correct now. Thierry
On 21.12.2017 17:10, Thierry Reding wrote: > On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote: >> On 21.12.2017 01:23, Dmitry Osipenko wrote: >>> On 21.12.2017 01:02, Thierry Reding wrote: >>>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote: >>>>> On 20.12.2017 23:16, Thierry Reding wrote: >>>>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote: >>>>>>> On 20.12.2017 21:01, Thierry Reding wrote: >>>>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote: >>>>>>>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke >>>>>>>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if >>>>>>>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with >>>>>>>>> both modesetting and opentegra drivers. On older Tegra's each plane has >>>>>>>>> a blending configuration which should be used to enable / disable alpha >>>>>>>>> blending and right now the blending configs are hardcoded to disabled >>>>>>>>> alpha blending. In order to support alpha formats properly, planes >>>>>>>>> blending configuration must be adjusted, until then the alpha formats >>>>>>>>> are equal to non-alpha. >>>>>>>>> >>>>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>>>>>>> --- >>>>>>>>> drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- >>>>>>>>> drivers/gpu/drm/tegra/dc.h | 1 + >>>>>>>>> drivers/gpu/drm/tegra/fb.c | 13 ------------- >>>>>>>>> drivers/gpu/drm/tegra/hub.c | 3 ++- >>>>>>>>> drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- >>>>>>>>> drivers/gpu/drm/tegra/plane.h | 2 +- >>>>>>>>> 6 files changed, 39 insertions(+), 31 deletions(-) >>>>>>>> >>>>>>>> This kept bugging me, so I spent some time looking at the blending >>>>>>>> programming. I came up with the attached patch which seems to work >>>>>>>> for all scenarios and is fairly similar to your patch. It has the >>>>>>>> added benefit that we can keep support for more formats. >>>>>>>> >>>>>>>> Any comments? >>>>>>>> >>>>>>>> Thierry >>>>>>>> --- >8 --- >>>>>>>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001 >>>>>>>> From: Thierry Reding <treding@nvidia.com> >>>>>>>> Date: Wed, 20 Dec 2017 09:39:14 +0100 >>>>>>>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending >>>>>>>> >>>>>>>> This implements alpha blending on legacy display controllers (Tegra20, >>>>>>>> Tegra30 and Tegra114). While it's theoretically possible to support the >>>>>>>> zpos property to enable userspace to specify the Z-order of each plane >>>>>>>> individually, this is not currently supported and the same fixed Z- >>>>>>>> order as previously defined is used. >>>>>>> >>>>>>> Perhaps one variant of implementing zpos could be by making overlays 'virtual', >>>>>>> so each virtual overlay will be backed by the real HW plane and we could swap >>>>>>> the HW planes of the virtual overlays, emulating zpos. >>>>>>> >>>>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since >>>>>>>> the opaque formats are now supported. >>>>>>>> >>>>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com> >>>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") >>>>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++--------- >>>>>>>> drivers/gpu/drm/tegra/dc.h | 13 ++++++++ >>>>>>>> drivers/gpu/drm/tegra/fb.c | 12 ------- >>>>>>>> drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++ >>>>>>>> drivers/gpu/drm/tegra/plane.h | 3 ++ >>>>>>>> 5 files changed, 116 insertions(+), 27 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c >>>>>>>> index bc65c314e00f..07c687d7f615 100644 >>>>>>>> --- a/drivers/gpu/drm/tegra/dc.c >>>>>>>> +++ b/drivers/gpu/drm/tegra/dc.c >>>>>>>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in) >>>>>>>> return dfixed_frac(inf); >>>>>>>> } >>>>>>>> >>>>>>>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane) >>>>>>>> +static void >>>>>>>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane, >>>>>>>> + const struct tegra_dc_window *window) >>>>>>>> { >>>>>>>> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) | >>>>>>>> + BLEND_COLOR_KEY_NONE; >>>>>>>> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) | >>>>>>>> + BLEND_COLOR_KEY_NONE; >>>>>>>> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255); >>>>>>>> + >>>>>>>> + /* enable alpha blending if window->alpha */ >>>>>>>> + if (window->alpha) { >>>>>>>> + background |= BLEND_CONTROL_DEPENDENT; >>>>>>>> + foreground |= BLEND_CONTROL_ALPHA; >>>>>>>> + } >>>>>>> >>>>>>> I think dependent weight means that window doesn't have alpha transparency. So >>>>>>> we should set the dependent_weight mode for opaque formats and alpha_weight for >>>>>>> formats with alpha channel. >>>>>> >>>>>> According to the TRM, dependent weight means that its weight will be 1 - >>>>>> the sum of the other overlapping windows. And it certainly seems to work >>>>>> that way in my tests (see below). >>>>> >>>>> Yes, and you are hardcoding the blending modes regardless of the actual plane >>>>> format. So even if underlying window has alpha format, it will be treated as it >>>>> is opaque. >>>> >>>> Ah yes, indeed. The patch currently assumes that if the current plane >>>> has an alpha component, then all the others will, too. That can probably >>>> be done fairly easily by looking at the current atomic state and >>>> inspecting the pixel format for each plane on the same CRTC. Let me take >>>> a stab at that tomorrow. >>> >>> Okay, please push patch to the public repo once it is ready and let me know. >>> I'll rebase cursor patch on it. >>> >>>> I'm wondering if we can deal with zpos, too, if we're already going >>>> through the trouble of looking at all planes involved. I think we can >>>> simply permute the WIN_BLEND register writes depending on the Z-order >>>> to implement proper zpos support. >>> I think we will have to permute the whole window state, not only the WIN_BLEND >>> register. >>> >>> Also I'm not sure how to make topmost window opaque and underlying windows >>> transparent, will have to check how blending works. Maybe it not possible at all.. >> >> Although it is simple to implement using the fixed weights for 2WIN cases, but >> then we will have to enhance the legacy blending code a tad to handle this case >> properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can >> still apply my patch that drops alpha formats on T20/30 and we will try to >> implement alpha + zpos for 4.17. > > I think I finally nailed this. Just sent out a v2 and updated my test > program to use different formats for each plane. I tested all opaque & > alpha combinations and it all looks correct now. Awesome, I'll test it soon. Please don't forget to correct the improper changes that you made to my "Link DC1 to DC0 on Tegra20" patch. See my replies to the mail from kbuild test bot. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 5299185cea2f..460510366bb8 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -299,12 +299,12 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index, } static const u32 tegra20_primary_formats[] = { - DRM_FORMAT_ARGB4444, - DRM_FORMAT_ARGB1555, DRM_FORMAT_RGB565, - DRM_FORMAT_RGBA5551, - DRM_FORMAT_ABGR8888, - DRM_FORMAT_ARGB8888, + /* non-native formats */ + DRM_FORMAT_XRGB1555, + DRM_FORMAT_RGBX5551, + DRM_FORMAT_XRGB8888, + DRM_FORMAT_XBGR8888, }; static const u32 tegra114_primary_formats[] = { @@ -369,7 +369,8 @@ static int tegra_plane_atomic_check(struct drm_plane *plane, err = tegra_plane_format(state->fb->format->format, &plane_state->format, - &plane_state->swap); + &plane_state->swap, + dc->soc->supports_opaque_formats); if (err < 0) return err; @@ -692,12 +693,12 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm, } static const u32 tegra20_overlay_formats[] = { - DRM_FORMAT_ARGB4444, - DRM_FORMAT_ARGB1555, DRM_FORMAT_RGB565, - DRM_FORMAT_RGBA5551, - DRM_FORMAT_ABGR8888, - DRM_FORMAT_ARGB8888, + /* non-native formats */ + DRM_FORMAT_XRGB1555, + DRM_FORMAT_RGBX5551, + DRM_FORMAT_XRGB8888, + DRM_FORMAT_XBGR8888, /* planar formats */ DRM_FORMAT_UYVY, DRM_FORMAT_YUYV, @@ -1854,6 +1855,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = { .primary_formats = tegra20_primary_formats, .num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats), .overlay_formats = tegra20_overlay_formats, + .supports_opaque_formats = false, }; static const struct tegra_dc_soc_info tegra30_dc_soc_info = { @@ -1869,6 +1871,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = { .primary_formats = tegra20_primary_formats, .num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats), .overlay_formats = tegra20_overlay_formats, + .supports_opaque_formats = false, }; static const struct tegra_dc_soc_info tegra114_dc_soc_info = { @@ -1884,6 +1887,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = { .primary_formats = tegra114_primary_formats, .num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats), .overlay_formats = tegra114_overlay_formats, + .supports_opaque_formats = true, }; static const struct tegra_dc_soc_info tegra124_dc_soc_info = { @@ -1899,6 +1903,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = { .primary_formats = tegra114_primary_formats, .num_overlay_formats = ARRAY_SIZE(tegra124_overlay_formats), .overlay_formats = tegra114_overlay_formats, + .supports_opaque_formats = true, }; static const struct tegra_dc_soc_info tegra210_dc_soc_info = { @@ -1914,6 +1919,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = { .primary_formats = tegra114_primary_formats, .num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats), .overlay_formats = tegra114_overlay_formats, + .supports_opaque_formats = true, }; static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = { @@ -1961,6 +1967,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = { .has_nvdisplay = true, .wgrps = tegra186_dc_wgrps, .num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps), + .supports_opaque_formats = true, }; static const struct of_device_id tegra_dc_of_match[] = { diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h index 8098f49c0d96..3a66a1127ee7 100644 --- a/drivers/gpu/drm/tegra/dc.h +++ b/drivers/gpu/drm/tegra/dc.h @@ -65,6 +65,7 @@ struct tegra_dc_soc_info { unsigned int num_primary_formats; const u32 *overlay_formats; unsigned int num_overlay_formats; + bool supports_opaque_formats; }; struct tegra_dc { diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index 5f6289c4e56a..bd8c9da1ed0d 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -253,19 +253,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper, cmd.height = sizes->surface_height; cmd.pitches[0] = round_up(sizes->surface_width * bytes_per_pixel, tegra->pitch_align); - - /* - * Early generations of Tegra (Tegra20 and Tegra30) do not support any - * of the X* or *X formats, only their A* or *A equivalents. Force the - * legacy framebuffer format to include an alpha component so that the - * framebuffer emulation can be supported on all generations. - */ - if (sizes->surface_bpp == 32 && sizes->surface_depth == 24) - sizes->surface_depth = 32; - - if (sizes->surface_bpp == 16 && sizes->surface_depth == 15) - sizes->surface_depth = 16; - cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c index f4911feda9ff..03298a1e87cc 100644 --- a/drivers/gpu/drm/tegra/hub.c +++ b/drivers/gpu/drm/tegra/hub.c @@ -332,7 +332,8 @@ static int tegra_shared_plane_atomic_check(struct drm_plane *plane, err = tegra_plane_format(state->fb->format->format, &plane_state->format, - &plane_state->swap); + &plane_state->swap, + true); if (err < 0) return err; diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index 326700cd0d80..345ec37c7481 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -103,7 +103,7 @@ int tegra_plane_state_add(struct tegra_plane *plane, return 0; } -int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap) +int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt) { /* assume no swapping of fetched data */ if (swap) @@ -147,11 +147,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap) break; case DRM_FORMAT_XRGB1555: - *format = WIN_COLOR_DEPTH_B5G5R5X1; + if (opaque_fmt) + *format = WIN_COLOR_DEPTH_B5G5R5X1; + else + *format = WIN_COLOR_DEPTH_B5G5R5A; break; case DRM_FORMAT_RGBX5551: - *format = WIN_COLOR_DEPTH_X1B5G5R5; + if (opaque_fmt) + *format = WIN_COLOR_DEPTH_X1B5G5R5; + else + *format = WIN_COLOR_DEPTH_AB5G5R5; break; case DRM_FORMAT_XBGR1555: @@ -175,11 +181,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap) break; case DRM_FORMAT_XRGB8888: - *format = WIN_COLOR_DEPTH_B8G8R8X8; + if (opaque_fmt) + *format = WIN_COLOR_DEPTH_B8G8R8X8; + else + *format = WIN_COLOR_DEPTH_B8G8R8A8; break; case DRM_FORMAT_XBGR8888: - *format = WIN_COLOR_DEPTH_R8G8B8X8; + if (opaque_fmt) + *format = WIN_COLOR_DEPTH_R8G8B8X8; + else + *format = WIN_COLOR_DEPTH_R8G8B8A8; break; case DRM_FORMAT_UYVY: diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h index fc7566f630fa..27145d64ad90 100644 --- a/drivers/gpu/drm/tegra/plane.h +++ b/drivers/gpu/drm/tegra/plane.h @@ -55,7 +55,7 @@ extern const struct drm_plane_funcs tegra_plane_funcs; int tegra_plane_state_add(struct tegra_plane *plane, struct drm_plane_state *state); -int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap); +int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt); bool tegra_plane_format_is_yuv(unsigned int format, bool *planar); #endif /* TEGRA_PLANE_H */
Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha. Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpu/drm/tegra/dc.c | 29 ++++++++++++++++++----------- drivers/gpu/drm/tegra/dc.h | 1 + drivers/gpu/drm/tegra/fb.c | 13 ------------- drivers/gpu/drm/tegra/hub.c | 3 ++- drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++----- drivers/gpu/drm/tegra/plane.h | 2 +- 6 files changed, 39 insertions(+), 31 deletions(-)