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