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

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

[mikita.lipski-5C7GfCeVMHo]

From: Mikita Lipski <mikita.lipski@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@amd.com>
---
 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];
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[mikita.lipski]

From: Mikita Lipski <mikita.lipski@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@amd.com>
---
 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];
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Kazlauskas, Nicholas]

On 2019-11-12 10:16 a.m., mikita.lipski@amd.com wrote:
> From: Mikita Lipski <mikita.lipski@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@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.

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];
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Kazlauskas, Nicholas]

On 2019-11-12 10:16 a.m., mikita.lipski@amd.com wrote:
> From: Mikita Lipski <mikita.lipski@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@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.

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];
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Harry Wentland]

On 2019-11-12 11:08 a.m., Kazlauskas, Nicholas wrote:
> On 2019-11-12 10:16 a.m., mikita.lipski@amd.com wrote:
>> From: Mikita Lipski <mikita.lipski@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@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@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];
>>
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Harry Wentland]

On 2019-11-12 11:08 a.m., Kazlauskas, Nicholas wrote:
> On 2019-11-12 10:16 a.m., mikita.lipski@amd.com wrote:
>> From: Mikita Lipski <mikita.lipski@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@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@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];
>>
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx