Applied. Thanks!
Alex
On Thu, Jun 23, 2022 at 9:34 AM Aurabindo Pillai
<aurabindo.pillai@xxxxxxx> wrote:
>
>
>
> On 2022-06-19 20:37, Bas Nieuwenhuizen wrote:
> > This reverts commit 5089c4a8ebea3c3ad9eedf038dad7098ebc06131.
> >
> > This breaks validation and enumeration of display capable modifiers.
> >
> > The early return true means the rest of the validation code never gets
> > executed, and we need that to enumerate the right modifiers to userspace
> > for the format.
> >
> > The modifiers that are in the initial list generated for a plane are the
> > superset for all formats and we need the proper checks in this function
> > to filter some of them out for formats with which they're invalid to be
> > used.
> >
> > Furthermore, the safety contract here is that we validate the incoming
> > modifiers to ensure the kernel can handle them and the display hardware
> > can handle them. This includes e.g. rejecting multi-plane images with DCC.
> >
> > Note that the legacy swizzle mechanism allows encoding more swizzles, and
> > at fb creation time we convert them to modifiers and reject those with
> > no corresponding modifiers. If we are seeing rejections I'm happy to
> > help define modifiers that correspond to those, or if absolutely needed
> > implement a fallback path to allow for less strict validation of the
> > legacy path.
> >
> > However, I'd like to revert this patch, since any of these is going to
> > be a significant rework of the patch, and I'd rather not the regression
> > gets into a release or forgotten in the meantime.
> >
> > Signed-off-by: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 53 +++----------------
> > 1 file changed, 7 insertions(+), 46 deletions(-)
> >
> > 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 60fb99b74713..698741fd39f4 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -4936,7 +4936,8 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,
> > {
> > struct amdgpu_device *adev = drm_to_adev(plane->dev);
> > const struct drm_format_info *info = drm_format_info(format);
> > - struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
> > + int i;
> > +
> > enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) & 3;
> >
> > if (!info)
> > @@ -4952,53 +4953,13 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane,
> > return true;
> > }
> >
> > - /* check if swizzle mode is supported by this version of DCN */
> > - switch (asic_id.chip_family) {
> > - case FAMILY_SI:
> > - case FAMILY_CI:
> > - case FAMILY_KV:
> > - case FAMILY_CZ:
> > - case FAMILY_VI:
> > - /* asics before AI does not have modifier support */
> > - return false;
> > - break;
> > - case FAMILY_AI:
> > - case FAMILY_RV:
> > - case FAMILY_NV:
> > - case FAMILY_VGH:
> > - case FAMILY_YELLOW_CARP:
> > - case AMDGPU_FAMILY_GC_10_3_6:
> > - case AMDGPU_FAMILY_GC_10_3_7:
> > - switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> > - case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_D:
> > - return true;
> > - break;
> > - default:
> > - return false;
> > - break;
> > - }
> > - break;
> > - case AMDGPU_FAMILY_GC_11_0_0:
> > - switch (AMD_FMT_MOD_GET(TILE, modifier)) {
> > - case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
> > - case AMD_FMT_MOD_TILE_GFX9_64K_D:
> > - return true;
> > - break;
> > - default:
> > - return false;
> > - break;
> > - }
> > - break;
> > - default:
> > - ASSERT(0); /* Unknown asic */
> > + /* Check that the modifier is on the list of the plane's supported modifiers. */
> > + for (i = 0; i < plane->modifier_count; i++) {
> > + if (modifier == plane->modifiers[i])
> > break;
> > }
> > + if (i == plane->modifier_count)
> > + return false;
> >
> > /*
> > * For D swizzle the canonical modifier depends on the bpp, so check
>
> We can expose an additional swizzle mode to work around the original
> problem with usermode which doesn't support modifiers but supports
> swizzle modes.
>
> Reviewed-by: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
