[PATCH 1/2] drm/amdgpu/virtual_dce: No need to pin the fb's bo

[Christian König]

Am 27.12.18 um 15:30 schrieb Michel Dänzer:
> On 2018-12-24 7:08 a.m., Emily Deng wrote:
>> For virtual display, no need to pin the fb's bo.
>>
>> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
>>
>> [...]
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index ea89c54..4e94473 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -188,10 +188,12 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
>>   		goto cleanup;
>>   	}
>>   
>> -	r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev));
>> -	if (unlikely(r != 0)) {
>> -		DRM_ERROR("failed to pin new abo buffer before flip\n");
>> -		goto unreserve;
>> +	if (!adev->enable_virtual_display) {
>> +		r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev));
>> +		if (unlikely(r != 0)) {
>> +			DRM_ERROR("failed to pin new abo buffer before flip\n");
>> +			goto unreserve;
>> +		}
>>   	}
>>   
>>   	r = amdgpu_ttm_alloc_gart(&new_abo->tbo);
> I think the amdgpu_ttm_alloc_gart call could be skipped as well?

Yup agree.

>
>
>> @@ -242,9 +245,10 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
>>   		goto cleanup;
>>   	}
>>   unpin:
>> -	if (unlikely(amdgpu_bo_unpin(new_abo) != 0)) {
>> -		DRM_ERROR("failed to unpin new abo in error path\n");
>> -	}
>> +	if (!adev->enable_virtual_display)
>> +		if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
>> +			DRM_ERROR("failed to unpin new abo in error path\n");
> This should be written as either
>
> 	if (!adev->enable_virtual_display &&
> 	    unlikely(amdgpu_bo_unpin(new_abo) != 0))
> 		DRM_ERROR("failed to unpin new abo in error path\n");
>
> or
>
> 	if (!adev->enable_virtual_display) {
> 		if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
> 			DRM_ERROR("failed to unpin new abo in error path\n");
> 	}
>
>
> P.S. A lot of people are out for Christmas and New Year's.

Indeed and thanks for the good explanation of the background in the 
other mail. I didn't realized that this was actually a bug fix for a pin 
imbalance.

BTW: It's good practice to note information like that in the commit message.

Anyway with the comment above fixed the series is Acked-by: Christian 
König <christian.koenig at amd.com>

Christian.