[PATCH] RFC: drm/amd/display: enable ABGR and XBGR formats (v2)
Mauro Rossi
issor.oruam at gmail.com
Tue Jul 17 18:38:43 UTC 2018
Hi Alex,
Il giorno mar 17 lug 2018 alle ore 15:43 Alex Deucher <alexdeucher at gmail.com>
ha scritto:
> On Sun, Jul 15, 2018 at 10:03 PM, Mauro Rossi <issor.oruam at gmail.com>
> wrote:
> > From: Mauro Rossi <issor.oruam at gmail.com>
> >
> > (v1) {A,X}BGR8888 code paths are added in amdgpu_dm, by using an
> fb_format
> > already listed in dc/dc_hw_types.h
> (SURFACE_PIXEL_FORMAT_GRPH_ABGR8888),
> > and in dce 8.0, 10.0 and 11.0, i.e. Bonaire and later.
> > GRPH_FORMAT_ARGB8888 is used due to lack of specific
> GRPH_FORMAT_ABGR8888
> >
> > (v2) support for {A,X}BGR8888 in atombios_crtc (now in dce4 path, to be
> refined)
> > to initialize frame buffer device and avoid following dmesg error:
> > "[drm] Cannot find any crtc or sizes"
> >
> > Tested with oreo-x86 (hwcomposer.drm + gralloc.gbm + mesa-dev/radv)
> > SurfaceFlinger can now select RGBA_8888 format for HWC_FRAMEBUFFER_TARGET
> > No major regression or crash observed so far, but some android 2D overlay
> > may be affected by color artifacts. Kind feedback requested.
> >
> > Signed-off-by: Mauro Rossi <issor.oruam at gmail.com>
>
> Please split the patch in three (one for radeon and one for amdgpu dc
> and one for amdgpu non-dc). Also the GRPH_SWAP_CONTROL register has a
> crossbar where you can change the channel routing. You may need that
> for the channel routing to work correctly.
>
> Alex
>
Thanks for your suggestion and guidance! :-)
I may need some time to assimilate the suggestions and some confirmations,
as I am an amateur in AMD GPU coding, to be honest, I should have mentioned
that before.
Regarding the radeon scope of changes,
do you recommend to keep the enablement of {A,X}BGR8888 for dce4 and later,
or to extend the enablement of {A,X}BGR8888 to older families of radeon
gpus/chipsets?
What is the lower radeon family where {A,X}BGR8888 can be natively
supported by HW,
by means of swap control registers for channel routing configuration?
Based on the scope of {A,X}BGR8888 support in final patches,
I may need to add handling in other dce code and maybe other modules,
could you please provide information in terms of necessary changes/high
level steps to follow?
Do you have some pointer to documentation on swap control registers for
the families
that may be considered as 'safe to be kept in scope' for {A,X}BGR8888
support?
Last but not least I would like to ask you about how to test no-regression,
even if this will come later,
when patches will be in good shape for further evaluation, do you have
tools and samples for conformance/no-regression testing?
I am asking because I don't have samples for all families.
Kind regards
Mauro
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 9 +++++++++
> > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 9 +++++++++
> > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 8 ++++++++
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++++++
> > drivers/gpu/drm/radeon/atombios_crtc.c | 8 ++++++++
> > 5 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > index 022f303463fc..d4280d2e7737 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > @@ -2005,6 +2005,15 @@ static int dce_v10_0_crtc_do_set_base(struct
> drm_crtc *crtc,
> > /* Greater 8 bpc fb needs to bypass hw-lut to retain
> precision */
> > bypass_lut = true;
> > break;
> > + case DRM_FORMAT_XBGR8888:
> > + case DRM_FORMAT_ABGR8888:
> > + fb_format = REG_SET_FIELD(0, GRPH_CONTROL, GRPH_DEPTH,
> 2);
> > + fb_format = REG_SET_FIELD(fb_format, GRPH_CONTROL,
> GRPH_FORMAT, 0); /* Hack */
> > +#ifdef __BIG_ENDIAN
> > + fb_swap = REG_SET_FIELD(fb_swap, GRPH_SWAP_CNTL,
> GRPH_ENDIAN_SWAP,
> > + ENDIAN_8IN32);
> > +#endif
> > + break;
> > default:
> > DRM_ERROR("Unsupported screen format %s\n",
> > drm_get_format_name(target_fb->format->format,
> &format_name));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > index 800a9f36ab4f..d48ee8f2e192 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > @@ -2044,6 +2044,15 @@ static int dce_v11_0_crtc_do_set_base(struct
> drm_crtc *crtc,
> > /* Greater 8 bpc fb needs to bypass hw-lut to retain
> precision */
> > bypass_lut = true;
> > break;
> > + case DRM_FORMAT_XBGR8888:
> > + case DRM_FORMAT_ABGR8888:
> > + fb_format = REG_SET_FIELD(0, GRPH_CONTROL, GRPH_DEPTH,
> 2);
> > + fb_format = REG_SET_FIELD(fb_format, GRPH_CONTROL,
> GRPH_FORMAT, 0); /* Hack */
> > +#ifdef __BIG_ENDIAN
> > + fb_swap = REG_SET_FIELD(fb_swap, GRPH_SWAP_CNTL,
> GRPH_ENDIAN_SWAP,
> > + ENDIAN_8IN32);
> > +#endif
> > + break;
> > default:
> > DRM_ERROR("Unsupported screen format %s\n",
> > drm_get_format_name(target_fb->format->format,
> &format_name));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > index 012e0a9ae0ff..0e2fc1ac475f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > @@ -1929,6 +1929,14 @@ static int dce_v8_0_crtc_do_set_base(struct
> drm_crtc *crtc,
> > /* Greater 8 bpc fb needs to bypass hw-lut to retain
> precision */
> > bypass_lut = true;
> > break;
> > + case DRM_FORMAT_XBGR8888:
> > + case DRM_FORMAT_ABGR8888:
> > + fb_format = ((GRPH_DEPTH_32BPP <<
> GRPH_CONTROL__GRPH_DEPTH__SHIFT) |
> > + (GRPH_FORMAT_ARGB8888 <<
> GRPH_CONTROL__GRPH_FORMAT__SHIFT)); /* Hack */
> > +#ifdef __BIG_ENDIAN
> > + fb_swap = (GRPH_ENDIAN_8IN32 <<
> GRPH_SWAP_CNTL__GRPH_ENDIAN_SWAP__SHIFT);
> > +#endif
> > + break;
> > default:
> > DRM_ERROR("Unsupported screen format %s\n",
> > drm_get_format_name(target_fb->format->format,
> &format_name));
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 63c67346d316..6c10fa291150 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1824,6 +1824,10 @@ static int fill_plane_attributes_from_fb(struct
> amdgpu_device *adev,
> > case DRM_FORMAT_ABGR2101010:
> > plane_state->format =
> SURFACE_PIXEL_FORMAT_GRPH_ABGR2101010;
> > break;
> > + case DRM_FORMAT_XBGR8888:
> > + case DRM_FORMAT_ABGR8888:
> > + plane_state->format = SURFACE_PIXEL_FORMAT_GRPH_ABGR8888;
> > + break;
> > case DRM_FORMAT_NV21:
> > plane_state->format =
> SURFACE_PIXEL_FORMAT_VIDEO_420_YCbCr;
> > break;
> > @@ -3115,6 +3119,8 @@ static const uint32_t rgb_formats[] = {
> > DRM_FORMAT_XBGR2101010,
> > DRM_FORMAT_ARGB2101010,
> > DRM_FORMAT_ABGR2101010,
> > + DRM_FORMAT_XBGR8888,
> > + DRM_FORMAT_ABGR8888,
> > };
> >
> > static const uint32_t yuv_formats[] = {
> > diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c
> b/drivers/gpu/drm/radeon/atombios_crtc.c
> > index 02baaaf20e9d..b954b3658a33 100644
> > --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> > +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> > @@ -1259,6 +1259,14 @@ static int dce4_crtc_do_set_base(struct drm_crtc
> *crtc,
> > /* Greater 8 bpc fb needs to bypass hw-lut to retain
> precision */
> > bypass_lut = true;
> > break;
> > + case DRM_FORMAT_XBGR8888:
> > + case DRM_FORMAT_ABGR8888:
> > + fb_format =
> (EVERGREEN_GRPH_DEPTH(EVERGREEN_GRPH_DEPTH_32BPP) |
> > +
> EVERGREEN_GRPH_FORMAT(EVERGREEN_GRPH_FORMAT_ARGB8888));
> > +#ifdef __BIG_ENDIAN
> > + fb_swap =
> EVERGREEN_GRPH_ENDIAN_SWAP(EVERGREEN_GRPH_ENDIAN_8IN32);
> > +#endif
> > + break;
> > default:
> > DRM_ERROR("Unsupported screen format %s\n",
> > drm_get_format_name(target_fb->format->format,
> &format_name));
> > --
> > 2.17.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180717/0bdec1d6/attachment.html>
More information about the amd-gfx
mailing list