[PATCH 1/2] drm/amd/display: Return correct error value

Harry Wentland hwentlan at amd.com
Tue Nov 12 20:14:28 UTC 2019


On 2019-11-12 11:08 a.m., Kazlauskas, Nicholas wrote:
> On 2019-11-12 10:16 a.m., mikita.lipski at amd.com wrote:
>> From: Mikita Lipski <mikita.lipski at amd.com>
>>
>> [why]
>> The function is expected to return instance of the timing generator
>> therefore we shouldn't be returning boolean in integer function,
>> and we shouldn't be returning zero so changing it to -1.
>>
>> Signed-off-by: Mikita Lipski <mikita.lipski at amd.com>
> 
> I wonder if some of these were intentional for returning 0. These lines
> were originally introduced for enabling seamless boot support with eDP
> and I think you're guaranteed to have those resources as instance 0.
> 

That sounds like an incorrect way of handling this. Mikita, can you
check, though, with the original authors (Anthony?) of this function and
make sure you get an ack from them?

If there's no objections from Anthony you can add my
Reviewed-by: Harry Wentland <harry.wentland at amd.com>

Harry

> Nicholas Kazlauskas
> 
>> ---
>>   drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
>> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
>> index 89b5f86cd40b..75cc58ecf647 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
>> @@ -1866,7 +1866,7 @@ static int acquire_resource_from_hw_enabled_state(
>>       inst = link->link_enc->funcs->get_dig_frontend(link->link_enc);
>>         if (inst == ENGINE_ID_UNKNOWN)
>> -        return false;
>> +        return -1;
>>         for (i = 0; i < pool->stream_enc_count; i++) {
>>           if (pool->stream_enc[i]->id == inst) {
>> @@ -1878,10 +1878,10 @@ static int
>> acquire_resource_from_hw_enabled_state(
>>         // tg_inst not found
>>       if (i == pool->stream_enc_count)
>> -        return false;
>> +        return -1;
>>         if (tg_inst >= pool->timing_generator_count)
>> -        return false;
>> +        return -1;
>>         if (!res_ctx->pipe_ctx[tg_inst].stream) {
>>           struct pipe_ctx *pipe_ctx = &res_ctx->pipe_ctx[tg_inst];
>>
> 


More information about the amd-gfx mailing list