[PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Stefan Agner]

In case the IOMMU API is not available compiling host1x fails with
the following error:
  In file included from drivers/gpu/host1x/hw/host1x06.c:27:
  drivers/gpu/host1x/hw/channel_hw.c: In function ‘host1x_channel_set_streamid’:
  drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit declaration of function
    ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?  [-Werror=implicit-function-declaration]
  struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
                              ^~~~~~~~~~~~~~~~~~~~
                              iommu_fwspec_free

Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/host1x/hw/channel_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
index 27101c04a827..4030d64916f0 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct host1x_job *job)
 
 static void host1x_channel_set_streamid(struct host1x_channel *channel)
 {
-#if HOST1X_HW >= 6
+#if IS_ENABLED(CONFIG_IOMMU_API) &&  HOST1X_HW >= 6
 	struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
 	u32 sid = spec ? spec->ids[0] & 0xffff : 0x7f;
 
-- 
2.21.0

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Ben Dooks]

On 2019-04-10 23:47, Stefan Agner wrote:
> In case the IOMMU API is not available compiling host1x fails with
> the following error:
>   In file included from drivers/gpu/host1x/hw/host1x06.c:27:
>   drivers/gpu/host1x/hw/channel_hw.c: In function 
> ‘host1x_channel_set_streamid’:
>   drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
> declaration of function
>     ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
> [-Werror=implicit-function-declaration]
>   struct iommu_fwspec *spec = 
> dev_iommu_fwspec_get(channel->dev->parent);
>                               ^~~~~~~~~~~~~~~~~~~~
>                               iommu_fwspec_free
> 
> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
> Signed-off-by: Stefan Agner <stefan@agner.ch>

would it be better to provide something like this i nthe header that
defines dev_iommu_fwspec_get() to be:

static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device 
*dev) { return NULL; }

although returning an PTR_ERR would have been better.

> ---
>  drivers/gpu/host1x/hw/channel_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/host1x/hw/channel_hw.c
> b/drivers/gpu/host1x/hw/channel_hw.c
> index 27101c04a827..4030d64916f0 100644
> --- a/drivers/gpu/host1x/hw/channel_hw.c
> +++ b/drivers/gpu/host1x/hw/channel_hw.c
> @@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct
> host1x_job *job)
> 
>  static void host1x_channel_set_streamid(struct host1x_channel 
> *channel)
>  {
> -#if HOST1X_HW >= 6
> +#if IS_ENABLED(CONFIG_IOMMU_API) &&  HOST1X_HW >= 6
>  	struct iommu_fwspec *spec = 
> dev_iommu_fwspec_get(channel->dev->parent);
>  	u32 sid = spec ? spec->ids[0] & 0xffff : 0x7f;

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]

On Thu, Apr 11, 2019 at 09:23:13AM +0100, Ben Dooks wrote:
> 
> 
> On 2019-04-10 23:47, Stefan Agner wrote:
> > In case the IOMMU API is not available compiling host1x fails with
> > the following error:
> >   In file included from drivers/gpu/host1x/hw/host1x06.c:27:
> >   drivers/gpu/host1x/hw/channel_hw.c: In function
> > ‘host1x_channel_set_streamid’:
> >   drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
> > declaration of function
> >     ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
> > [-Werror=implicit-function-declaration]
> >   struct iommu_fwspec *spec =
> > dev_iommu_fwspec_get(channel->dev->parent);
> >                               ^~~~~~~~~~~~~~~~~~~~
> >                               iommu_fwspec_free
> > 
> > Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> would it be better to provide something like this i nthe header that
> defines dev_iommu_fwspec_get() to be:
> 
> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
> { return NULL; }
> 
> although returning an PTR_ERR would have been better.

I don't think there's really a large number of failures here. Either
your device has an IOMMU fwspec or it doesn't.

But yes, I think it'd be better to have the above static inline dummy in
iommu.h, but I'll apply this for now in the hopes of getting it in
before v5.1 final.

Thierry

> 
> > ---
> >  drivers/gpu/host1x/hw/channel_hw.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/host1x/hw/channel_hw.c
> > b/drivers/gpu/host1x/hw/channel_hw.c
> > index 27101c04a827..4030d64916f0 100644
> > --- a/drivers/gpu/host1x/hw/channel_hw.c
> > +++ b/drivers/gpu/host1x/hw/channel_hw.c
> > @@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct
> > host1x_job *job)
> > 
> >  static void host1x_channel_set_streamid(struct host1x_channel *channel)
> >  {
> > -#if HOST1X_HW >= 6
> > +#if IS_ENABLED(CONFIG_IOMMU_API) &&  HOST1X_HW >= 6
> >  	struct iommu_fwspec *spec =
> > dev_iommu_fwspec_get(channel->dev->parent);
> >  	u32 sid = spec ? spec->ids[0] & 0xffff : 0x7f;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Mikko Perttunen]

On 11.4.2019 11.30, Thierry Reding wrote:
> On Thu, Apr 11, 2019 at 09:23:13AM +0100, Ben Dooks wrote:
>>
>>
>> On 2019-04-10 23:47, Stefan Agner wrote:
>>> In case the IOMMU API is not available compiling host1x fails with
>>> the following error:
>>>    In file included from drivers/gpu/host1x/hw/host1x06.c:27:
>>>    drivers/gpu/host1x/hw/channel_hw.c: In function
>>> ‘host1x_channel_set_streamid’:
>>>    drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
>>> declaration of function
>>>      ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
>>> [-Werror=implicit-function-declaration]
>>>    struct iommu_fwspec *spec =
>>> dev_iommu_fwspec_get(channel->dev->parent);
>>>                                ^~~~~~~~~~~~~~~~~~~~
>>>                                iommu_fwspec_free
>>>
>>> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>
>> would it be better to provide something like this i nthe header that
>> defines dev_iommu_fwspec_get() to be:
>>
>> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
>> { return NULL; }
>>
>> although returning an PTR_ERR would have been better.
> 
> I don't think there's really a large number of failures here. Either
> your device has an IOMMU fwspec or it doesn't.
> 
> But yes, I think it'd be better to have the above static inline dummy in
> iommu.h, but I'll apply this for now in the hopes of getting it in
> before v5.1 final.

A similar patch was already sent before by someone. That one also 
programs the bypass stream ID (0x7f) even if IOMMU is disabled. We 
should pick that patch instead.

Thanks,
Mikko

> 
> Thierry >
>>
>>> ---
>>>   drivers/gpu/host1x/hw/channel_hw.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c
>>> b/drivers/gpu/host1x/hw/channel_hw.c
>>> index 27101c04a827..4030d64916f0 100644
>>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>>> @@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct
>>> host1x_job *job)
>>>
>>>   static void host1x_channel_set_streamid(struct host1x_channel *channel)
>>>   {
>>> -#if HOST1X_HW >= 6
>>> +#if IS_ENABLED(CONFIG_IOMMU_API) &&  HOST1X_HW >= 6
>>>   	struct iommu_fwspec *spec =
>>> dev_iommu_fwspec_get(channel->dev->parent);
>>>   	u32 sid = spec ? spec->ids[0] & 0xffff : 0x7f;
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Stefan Agner]

On 11.04.2019 10:23, Ben Dooks wrote:
> On 2019-04-10 23:47, Stefan Agner wrote:
>> In case the IOMMU API is not available compiling host1x fails with
>> the following error:
>>   In file included from drivers/gpu/host1x/hw/host1x06.c:27:
>>   drivers/gpu/host1x/hw/channel_hw.c: In function ‘host1x_channel_set_streamid’:
>>   drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
>> declaration of function
>>     ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
>> [-Werror=implicit-function-declaration]
>>   struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
>>                               ^~~~~~~~~~~~~~~~~~~~
>>                               iommu_fwspec_free
>>
>> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> would it be better to provide something like this i nthe header that
> defines dev_iommu_fwspec_get() to be:
> 
> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device
> *dev) { return NULL; }
> 
> although returning an PTR_ERR would have been better.
> 

The problem is that the code also accesses fields of struct iommu_fwspec
which are not defined in the !CONFIG_IOMMU_API case.

--
Stefan

>> ---
>>  drivers/gpu/host1x/hw/channel_hw.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c
>> b/drivers/gpu/host1x/hw/channel_hw.c
>> index 27101c04a827..4030d64916f0 100644
>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>> @@ -114,7 +114,7 @@ static inline void synchronize_syncpt_base(struct
>> host1x_job *job)
>>
>>  static void host1x_channel_set_streamid(struct host1x_channel *channel)
>>  {
>> -#if HOST1X_HW >= 6
>> +#if IS_ENABLED(CONFIG_IOMMU_API) &&  HOST1X_HW >= 6
>>  	struct iommu_fwspec *spec = dev_iommu_fwspec_get(channel->dev->parent);
>>  	u32 sid = spec ? spec->ids[0] & 0xffff : 0x7f;

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Dmitry Osipenko]

11.04.2019 13:06, Mikko Perttunen пишет:
> On 11.4.2019 11.30, Thierry Reding wrote:
>> On Thu, Apr 11, 2019 at 09:23:13AM +0100, Ben Dooks wrote:
>>>
>>>
>>> On 2019-04-10 23:47, Stefan Agner wrote:
>>>> In case the IOMMU API is not available compiling host1x fails with
>>>> the following error:
>>>>    In file included from drivers/gpu/host1x/hw/host1x06.c:27:
>>>>    drivers/gpu/host1x/hw/channel_hw.c: In function
>>>> ‘host1x_channel_set_streamid’:
>>>>    drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
>>>> declaration of function
>>>>      ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
>>>> [-Werror=implicit-function-declaration]
>>>>    struct iommu_fwspec *spec =
>>>> dev_iommu_fwspec_get(channel->dev->parent);
>>>>                                ^~~~~~~~~~~~~~~~~~~~
>>>>                                iommu_fwspec_free
>>>>
>>>> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>
>>> would it be better to provide something like this i nthe header that
>>> defines dev_iommu_fwspec_get() to be:
>>>
>>> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
>>> { return NULL; }
>>>
>>> although returning an PTR_ERR would have been better.
>>
>> I don't think there's really a large number of failures here. Either
>> your device has an IOMMU fwspec or it doesn't.
>>
>> But yes, I think it'd be better to have the above static inline dummy in
>> iommu.h, but I'll apply this for now in the hopes of getting it in
>> before v5.1 final.
> 
> A similar patch was already sent before by someone. That one also programs the bypass stream ID (0x7f) even if IOMMU is disabled. We should pick that patch instead.

For the record.. here is that patch https://patchwork.ozlabs.org/patch/1052364/

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2176 bytes --]

On Thu, Apr 11, 2019 at 02:31:48PM +0300, Dmitry Osipenko wrote:
> 11.04.2019 13:06, Mikko Perttunen пишет:
> > On 11.4.2019 11.30, Thierry Reding wrote:
> >> On Thu, Apr 11, 2019 at 09:23:13AM +0100, Ben Dooks wrote:
> >>>
> >>>
> >>> On 2019-04-10 23:47, Stefan Agner wrote:
> >>>> In case the IOMMU API is not available compiling host1x fails with
> >>>> the following error:
> >>>>    In file included from drivers/gpu/host1x/hw/host1x06.c:27:
> >>>>    drivers/gpu/host1x/hw/channel_hw.c: In function
> >>>> ‘host1x_channel_set_streamid’:
> >>>>    drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
> >>>> declaration of function
> >>>>      ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
> >>>> [-Werror=implicit-function-declaration]
> >>>>    struct iommu_fwspec *spec =
> >>>> dev_iommu_fwspec_get(channel->dev->parent);
> >>>>                                ^~~~~~~~~~~~~~~~~~~~
> >>>>                                iommu_fwspec_free
> >>>>
> >>>> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
> >>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >>>
> >>> would it be better to provide something like this i nthe header that
> >>> defines dev_iommu_fwspec_get() to be:
> >>>
> >>> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
> >>> { return NULL; }
> >>>
> >>> although returning an PTR_ERR would have been better.
> >>
> >> I don't think there's really a large number of failures here. Either
> >> your device has an IOMMU fwspec or it doesn't.
> >>
> >> But yes, I think it'd be better to have the above static inline dummy in
> >> iommu.h, but I'll apply this for now in the hopes of getting it in
> >> before v5.1 final.
> > 
> > A similar patch was already sent before by someone. That one also programs the bypass stream ID (0x7f) even if IOMMU is disabled. We should pick that patch instead.
> 
> For the record.. here is that patch https://patchwork.ozlabs.org/patch/1052364/

Ugh... too late. I'll apply Arnd's patch on top of this one.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu: host1x: fix compile error when IOMMU API is not available

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On Thu, Apr 11, 2019 at 02:31:48PM +0300, Dmitry Osipenko wrote:
> 11.04.2019 13:06, Mikko Perttunen пишет:
> > On 11.4.2019 11.30, Thierry Reding wrote:
> >> On Thu, Apr 11, 2019 at 09:23:13AM +0100, Ben Dooks wrote:
> >>>
> >>>
> >>> On 2019-04-10 23:47, Stefan Agner wrote:
> >>>> In case the IOMMU API is not available compiling host1x fails with
> >>>> the following error:
> >>>>    In file included from drivers/gpu/host1x/hw/host1x06.c:27:
> >>>>    drivers/gpu/host1x/hw/channel_hw.c: In function
> >>>> ‘host1x_channel_set_streamid’:
> >>>>    drivers/gpu/host1x/hw/channel_hw.c:118:30: error: implicit
> >>>> declaration of function
> >>>>      ‘dev_iommu_fwspec_get’; did you mean ‘iommu_fwspec_free’?
> >>>> [-Werror=implicit-function-declaration]
> >>>>    struct iommu_fwspec *spec =
> >>>> dev_iommu_fwspec_get(channel->dev->parent);
> >>>>                                ^~~~~~~~~~~~~~~~~~~~
> >>>>                                iommu_fwspec_free
> >>>>
> >>>> Fixes: de5469c21ff9 ("gpu: host1x: Program the channel stream ID")
> >>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >>>
> >>> would it be better to provide something like this i nthe header that
> >>> defines dev_iommu_fwspec_get() to be:
> >>>
> >>> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
> >>> { return NULL; }
> >>>
> >>> although returning an PTR_ERR would have been better.
> >>
> >> I don't think there's really a large number of failures here. Either
> >> your device has an IOMMU fwspec or it doesn't.
> >>
> >> But yes, I think it'd be better to have the above static inline dummy in
> >> iommu.h, but I'll apply this for now in the hopes of getting it in
> >> before v5.1 final.
> > 
> > A similar patch was already sent before by someone. That one also programs the bypass stream ID (0x7f) even if IOMMU is disabled. We should pick that patch instead.
> 
> For the record.. here is that patch https://patchwork.ozlabs.org/patch/1052364/

Ugh... too late. I'll apply Arnd's patch on top of this one.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]