2018-09-21 21:56:32

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] drm/amd/display: Change status's type in aux_reply_transaction_data

Clang warns when one enumerated type is implicitly converted to another.

drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
implicit conversion from enumeration type 'enum
aux_channel_operation_result' to different enumeration type 'enum
aux_transaction_reply' [-Wenum-conversion]
reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
warning: implicit conversion from enumeration type 'enum
aux_channel_operation_result' to different enumeration type 'enum
aux_transaction_reply' [-Wenum-conversion]
reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Instead of implicitly or explicitly converting between types, just
change status to type uint8_t (since its max size is 255) which avoids
this construct altogether.

Reported-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
index 05c8c31d8b31..97e1d4d19263 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
@@ -79,7 +79,7 @@ enum aux_transaction_reply {
};

struct aux_reply_transaction_data {
- enum aux_transaction_reply status;
+ uint8_t status;
uint32_t length;
uint8_t *data;
};
--
2.19.0



2018-09-24 22:10:02

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/display: Change status's type in aux_reply_transaction_data

On Fri, Sep 21, 2018 at 2:55 PM Nathan Chancellor
<[email protected]> wrote:
>
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
> implicit conversion from enumeration type 'enum
> aux_channel_operation_result' to different enumeration type 'enum
> aux_transaction_reply' [-Wenum-conversion]
> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
> warning: implicit conversion from enumeration type 'enum
> aux_channel_operation_result' to different enumeration type 'enum
> aux_transaction_reply' [-Wenum-conversion]
> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I think the enum is actually wrong here. I think the correct fix would be:

- reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
+ reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;

The identifiers are so similar, my guess was that it was easy to mix
them up. This looks like an actual bug to me, since the identifiers
have different values between the 2 different enums.

>
> Instead of implicitly or explicitly converting between types, just
> change status to type uint8_t (since its max size is 255) which avoids
> this construct altogether.
>
> Reported-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> index 05c8c31d8b31..97e1d4d19263 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> @@ -79,7 +79,7 @@ enum aux_transaction_reply {
> };
>
> struct aux_reply_transaction_data {
> - enum aux_transaction_reply status;
> + uint8_t status;
> uint32_t length;
> uint8_t *data;
> };
> --
> 2.19.0
>


--
Thanks,
~Nick Desaulniers

2018-09-24 22:23:55

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/display: Change status's type in aux_reply_transaction_data

On Mon, Sep 24, 2018 at 03:07:16PM -0700, Nick Desaulniers wrote:
> On Fri, Sep 21, 2018 at 2:55 PM Nathan Chancellor
> <[email protected]> wrote:
> >
> > Clang warns when one enumerated type is implicitly converted to another.
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
> > implicit conversion from enumeration type 'enum
> > aux_channel_operation_result' to different enumeration type 'enum
> > aux_transaction_reply' [-Wenum-conversion]
> > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
> > warning: implicit conversion from enumeration type 'enum
> > aux_channel_operation_result' to different enumeration type 'enum
> > aux_transaction_reply' [-Wenum-conversion]
> > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> I think the enum is actually wrong here. I think the correct fix would be:
>
> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
>
> The identifiers are so similar, my guess was that it was easy to mix
> them up. This looks like an actual bug to me, since the identifiers
> have different values between the 2 different enums.
>

Hmmm interesting... I will be happy to send a v2 with your suggestion if
one of the maintainers could confirm that to be the case (given DRM code
is rather dense).

Thanks for the review!
Nathan

> >
> > Instead of implicitly or explicitly converting between types, just
> > change status to type uint8_t (since its max size is 255) which avoids
> > this construct altogether.
> >
> > Reported-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> > drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> > index 05c8c31d8b31..97e1d4d19263 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
> > @@ -79,7 +79,7 @@ enum aux_transaction_reply {
> > };
> >
> > struct aux_reply_transaction_data {
> > - enum aux_transaction_reply status;
> > + uint8_t status;
> > uint32_t length;
> > uint8_t *data;
> > };
> > --
> > 2.19.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers

2018-09-27 10:05:28

by Harry Wentland

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/display: Change status's type in aux_reply_transaction_data

On 2018-09-24 06:22 PM, Nathan Chancellor wrote:
> On Mon, Sep 24, 2018 at 03:07:16PM -0700, Nick Desaulniers wrote:
>> On Fri, Sep 21, 2018 at 2:55 PM Nathan Chancellor
>> <[email protected]> wrote:
>>>
>>> Clang warns when one enumerated type is implicitly converted to another.
>>>
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
>>> implicit conversion from enumeration type 'enum
>>> aux_channel_operation_result' to different enumeration type 'enum
>>> aux_transaction_reply' [-Wenum-conversion]
>>> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
>>> warning: implicit conversion from enumeration type 'enum
>>> aux_channel_operation_result' to different enumeration type 'enum
>>> aux_transaction_reply' [-Wenum-conversion]
>>> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> I think the enum is actually wrong here. I think the correct fix would be:
>>
>> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
>>
>> The identifiers are so similar, my guess was that it was easy to mix
>> them up. This looks like an actual bug to me, since the identifiers
>> have different values between the 2 different enums.
>>
>
> Hmmm interesting... I will be happy to send a v2 with your suggestion if
> one of the maintainers could confirm that to be the case (given DRM code
> is rather dense).
>

Nick is correct. We should keep the enum but assign AUX_TRANSACTION_REPLY_HPD_DISCON in dce_aux.c and aux_engine_dce110.c.

Thanks for spotting this.

Harry

> Thanks for the review!
> Nathan
>
>>>
>>> Instead of implicitly or explicitly converting between types, just
>>> change status to type uint8_t (since its max size is 255) which avoids
>>> this construct altogether.
>>>
>>> Reported-by: Nick Desaulniers <[email protected]>
>>> Signed-off-by: Nathan Chancellor <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
>>> index 05c8c31d8b31..97e1d4d19263 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
>>> @@ -79,7 +79,7 @@ enum aux_transaction_reply {
>>> };
>>>
>>> struct aux_reply_transaction_data {
>>> - enum aux_transaction_reply status;
>>> + uint8_t status;
>>> uint32_t length;
>>> uint8_t *data;
>>> };
>>> --
>>> 2.19.0
>>>
>>
>>
>> --
>> Thanks,
>> ~Nick Desaulniers

2018-09-27 18:07:47

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2] drm/amd/display: Use proper enums in process_channel_reply

Clang warns when one enumerated type is implicitly converted to another.

drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
implicit conversion from enumeration type 'enum
aux_channel_operation_result' to different enumeration type 'enum
aux_transaction_reply' [-Wenum-conversion]
reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
warning: implicit conversion from enumeration type 'enum
aux_channel_operation_result' to different enumeration type 'enum
aux_transaction_reply' [-Wenum-conversion]
reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The current enum is incorrect, it should be from aux_transaction_reply,
so use AUX_TRANSACTION_REPLY_HPD_DISCON.

Reported-by: Nick Desaulniers <[email protected]>
Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---

v1 -> v2:

* Rather than change status to an integer, use the proper enumerated
type from aux_transaction reply as suggested by Nick and confirmed
by Henry.

drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +-
.../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
index 3f5b2e6f7553..aaeb7faac0c4 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
@@ -312,7 +312,7 @@ static void process_channel_reply(

/* in case HPD is LOW, exit AUX transaction */
if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
- reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
+ reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
return;
}

diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
index 8eee8ace1259..59c3ed43d609 100644
--- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
+++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
@@ -346,7 +346,7 @@ static void process_channel_reply(

/* in case HPD is LOW, exit AUX transaction */
if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
- reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
+ reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
return;
}

--
2.19.0


2018-09-27 18:09:22

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amd/display: Use proper enums in process_channel_reply

On Thu, Sep 27, 2018 at 11:06:33AM -0700, Nathan Chancellor wrote:
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
> implicit conversion from enumeration type 'enum
> aux_channel_operation_result' to different enumeration type 'enum
> aux_transaction_reply' [-Wenum-conversion]
> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
> warning: implicit conversion from enumeration type 'enum
> aux_channel_operation_result' to different enumeration type 'enum
> aux_transaction_reply' [-Wenum-conversion]
> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The current enum is incorrect, it should be from aux_transaction_reply,
> so use AUX_TRANSACTION_REPLY_HPD_DISCON.
>
> Reported-by: Nick Desaulniers <[email protected]>
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
>
> v1 -> v2:
>
> * Rather than change status to an integer, use the proper enumerated
> type from aux_transaction reply as suggested by Nick and confirmed
> by Henry.

Sigh Harry, sorry...

>
> drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +-
> .../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> index 3f5b2e6f7553..aaeb7faac0c4 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> @@ -312,7 +312,7 @@ static void process_channel_reply(
>
> /* in case HPD is LOW, exit AUX transaction */
> if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
> return;
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> index 8eee8ace1259..59c3ed43d609 100644
> --- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> +++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> @@ -346,7 +346,7 @@ static void process_channel_reply(
>
> /* in case HPD is LOW, exit AUX transaction */
> if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
> return;
> }
>
> --
> 2.19.0
>

2018-09-27 18:12:55

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amd/display: Use proper enums in process_channel_reply

On Thu, Sep 27, 2018 at 11:08 AM Nathan Chancellor
<[email protected]> wrote:
>
> On Thu, Sep 27, 2018 at 11:06:33AM -0700, Nathan Chancellor wrote:
> > Clang warns when one enumerated type is implicitly converted to another.
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
> > implicit conversion from enumeration type 'enum
> > aux_channel_operation_result' to different enumeration type 'enum
> > aux_transaction_reply' [-Wenum-conversion]
> > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
> > warning: implicit conversion from enumeration type 'enum
> > aux_channel_operation_result' to different enumeration type 'enum
> > aux_transaction_reply' [-Wenum-conversion]
> > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > The current enum is incorrect, it should be from aux_transaction_reply,
> > so use AUX_TRANSACTION_REPLY_HPD_DISCON.
> >
> > Reported-by: Nick Desaulniers <[email protected]>
> > Suggested-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Nathan Chancellor <[email protected]>

Reviewed-by: Nick Desaulniers <[email protected]>

> > ---
> >
> > v1 -> v2:
> >
> > * Rather than change status to an integer, use the proper enumerated
> > type from aux_transaction reply as suggested by Nick and confirmed
> > by Henry.
>
> Sigh Harry, sorry...

Thanks for the patch, Nathan. Don't worry about sending a v3 over
this; its below the commit line so this detail wont get committed.

>
> >
> > drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +-
> > .../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> > index 3f5b2e6f7553..aaeb7faac0c4 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
> > @@ -312,7 +312,7 @@ static void process_channel_reply(
> >
> > /* in case HPD is LOW, exit AUX transaction */
> > if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
> > - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
> > return;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> > index 8eee8ace1259..59c3ed43d609 100644
> > --- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> > +++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
> > @@ -346,7 +346,7 @@ static void process_channel_reply(
> >
> > /* in case HPD is LOW, exit AUX transaction */
> > if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
> > - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
> > + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
> > return;
> > }
> >
> > --
> > 2.19.0
> >



--
Thanks,
~Nick Desaulniers

2018-10-02 14:56:36

by Harry Wentland

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amd/display: Use proper enums in process_channel_reply

On 2018-09-27 02:11 PM, Nick Desaulniers wrote:
> On Thu, Sep 27, 2018 at 11:08 AM Nathan Chancellor
> <[email protected]> wrote:
>>
>> On Thu, Sep 27, 2018 at 11:06:33AM -0700, Nathan Chancellor wrote:
>>> Clang warns when one enumerated type is implicitly converted to another.
>>>
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
>>> implicit conversion from enumeration type 'enum
>>> aux_channel_operation_result' to different enumeration type 'enum
>>> aux_transaction_reply' [-Wenum-conversion]
>>> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
>>> warning: implicit conversion from enumeration type 'enum
>>> aux_channel_operation_result' to different enumeration type 'enum
>>> aux_transaction_reply' [-Wenum-conversion]
>>> reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> The current enum is incorrect, it should be from aux_transaction_reply,
>>> so use AUX_TRANSACTION_REPLY_HPD_DISCON.
>>>
>>> Reported-by: Nick Desaulniers <[email protected]>
>>> Suggested-by: Nick Desaulniers <[email protected]>
>>> Signed-off-by: Nathan Chancellor <[email protected]>
>
> Reviewed-by: Nick Desaulniers <[email protected]>

Reviewed-by: Harry Wentland <[email protected]>

Pulling it in through amd-staging-drm-next.

>
>>> ---
>>>
>>> v1 -> v2:
>>>
>>> * Rather than change status to an integer, use the proper enumerated
>>> type from aux_transaction reply as suggested by Nick and confirmed
>>> by Henry.
>>
>> Sigh Harry, sorry...
>

No worries. You're not the first to get that wrong. :)

Harry

> Thanks for the patch, Nathan. Don't worry about sending a v3 over
> this; its below the commit line so this detail wont get committed.
>
>>
>>>
>>> drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +-
>>> .../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
>>> index 3f5b2e6f7553..aaeb7faac0c4 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
>>> @@ -312,7 +312,7 @@ static void process_channel_reply(
>>>
>>> /* in case HPD is LOW, exit AUX transaction */
>>> if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
>>> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
>>> return;
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
>>> index 8eee8ace1259..59c3ed43d609 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
>>> @@ -346,7 +346,7 @@ static void process_channel_reply(
>>>
>>> /* in case HPD is LOW, exit AUX transaction */
>>> if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
>>> - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
>>> + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
>>> return;
>>> }
>>>
>>> --
>>> 2.19.0
>>>
>
>
>