Re: [PATCH 06/18] drm/amd/display: Drop underlay plane support

From: "Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>
To: Nicholas Kazlauskas <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 06/18] drm/amd/display: Drop underlay plane support
Date: Tue, 5 Mar 2019 18:09:04 +0100	[thread overview]
Message-ID: <daf986ad-a939-05d2-8ee5-e103727d6b1d@daenzer.net> (raw)
In-Reply-To: <20190225224615.4507-7-Bhawanpreet.Lakha-5C7GfCeVMHo@public.gmane.org>

On 2019-02-25 11:46 p.m., Bhawanpreet Lakha wrote:
> From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> [Why]
> Primary and underlay planes were previously exposed to DRM by using
> max_planes and max_slave_planes.
> 
> The value for max_planes was always pipe_count + has_underlay.
> If there was an underlay pipe, then max_slave_planes = 1.
> 
> Raven has pipe_count = 4, max_planes = 4, and max_slave_planes = 1.
> So during plane initialziation it was actually "creating"
> 1 overlay plane and 3 primary planes... or it would be, had its
> plane_type array not been dm_plane_type_default, which will only create
> DRM_PLANE_TYPE_PRIMARY planes.
> 
> We can expose primary planes as supporting more than one CRTC at a time
> to more closely resemble plane behavior on DCN but userspace doesn't
> really expect planes to be used in this manner and will either
> ignore the planes or crash.
> 
> Planes with index greater than max_streams are marked as supporting
> all CRTCs. No ASIC currently has primary plane count greater than the
> stream count but we shouldn't expose more than necessary.
> 
> [How]
> Drop support for underlay planes. They aren't well tested and don't
> fully work right at the moment.
> 
> Only create one primary plane per CRTC so we're not creating overlays.
> 
> Initialize plane types directly instead of referencing a misleading
> array of plane types.
> 
> Change-Id: Ibdf1caa8a2be42df6f7574dae97c49734fa44151
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Reviewed-by: Tony Cheng <Tony.Cheng@amd.com>
> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> 
> 
> [...]
> 
> @@ -1925,21 +1901,17 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>  		return -EINVAL;
>  	}
>  
> -	/* Identify the number of planes to be initialized */
> -	total_overlay_planes = dm->dc->caps.max_slave_planes;
> -	total_primary_planes = dm->dc->caps.max_planes - dm->dc->caps.max_slave_planes;
> +	/* There is one primary plane per CRTC */
> +	primary_planes = dm->dc->caps.max_streams;
> +	ASSERT(primary_planes < AMDGPU_MAX_PLANES);

This assertion fails for me with Bonaire. In fact, since
AMDGPU_MAX_PLANES is 6, and primary_planes seems to be the number of
CRTCs, it'll fail with most GPUs?

Maybe it was meant to be <= instead of < ?

-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx