[PATCH 6/7] Revert "drm/amd/display: Implement zpos property"

[Leo Li]

On 2023-08-18 04:25, Melissa Wen wrote:
> On 07/26, Aurabindo Pillai wrote:
>> This reverts commit 6c8ff1683d30024c8cff137d30b740a405cc084e.
>>
>> This patch causes IGT test case 'kms_atomic at plane-immutable-zpos' to
>> fail on AMDGPU hardware.
>>
>> Cc: Joshua Ashton <joshua at froggi.es>
>> Signed-off-by: Nicholas Choi <Nicholas.Choi at amd.com>
>> ---
>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
>> index 2198df96ed6f..8eeca160d434 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
>> @@ -1468,15 +1468,6 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>>   		drm_plane_create_blend_mode_property(plane, blend_caps);
>>   	}
>>   
>> -	if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
>> -		drm_plane_create_zpos_immutable_property(plane, 0);
>> -	} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
>> -		unsigned int zpos = 1 + drm_plane_index(plane);
>> -		drm_plane_create_zpos_property(plane, zpos, 1, 254);
>> -	} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
>> -		drm_plane_create_zpos_immutable_property(plane, 255);
>> -	}
> 
> Hi Jay and Nicholas,
> 
> I'm examining this regression and, looking at the test code, I consider
> this failure is caused by an incorrect assumption in the IGT test in
> which any zpos values must be in the normalized range of 0 and N planes
> per CRTC.
> 
> 	for (int k = 0; k < n_planes; k++) {
> 		int zpos;
> 		igt_plane_t *temp;
> 
> 		temp = &display->pipes[pipe->pipe].planes[k];
> 
> 		if (!igt_plane_has_prop(temp, IGT_PLANE_ZPOS))
> 			continue;
> 
> 		zpos = igt_plane_get_prop(temp, IGT_PLANE_ZPOS);
> 
> 		igt_assert_lt(zpos, n_planes);  // test case fails here
> 
> 		plane_ptr[zpos] = temp;
> 	}
> 
> 
> I didn't find anything in the DRM documentation that imposes this
> behavior. Also, the plane composition in the test is working correctly
> with this patch and without this likely-misleading assert. In addition,
> enabling zpos property increases the coverage of
> `kms_atomic at plane-immutable-zpos` test (previously this part of the test
> was just bypassed), so it's not a regression per se. Therefore, I'm
> inclined to send a fix to IGT, instead of implementing a behavior that
> fit the test but may not fit well AMD display machinery.
> 
> But first I wonder if you guys find any other test or visual check that
> fail with this feature?
> 
> I checked other IGT KMS plane tests and AMD MPO tests (in `amd_plane`)
> and results are preserved. If there are no other issues besides IGT
> plane-immutable-zpos, I'll proceed with sending the change to IGT.
> 
> Thanks,
> 
> Melissa

Hi Melissa,

Thanks for taking a look at the IGT test. Looks like the IGT failures
are the only concerns, and I agree that it doesn't make sense to require
zpos to be normalized between 0 and number of planes.

Thanks,
Leo

> 
>> -
>>   	if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
>>   	    plane_cap &&
>>   	    (plane_cap->pixel_format_support.nv12 ||
>> -- 
>> 2.41.0
>>