2022-03-25 17:45:55

by Christian König

Subject: Re: [PATCH v2] drm/qxl: fix qxl can't use in arm64

Am 24.03.22 um 11:49 schrieb Cong Liu:
> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> access. and qxl is a virtual device, it can be treated more like RAM
> than actual MMIO registers. use ioremap_wc() replace it.
>
> Signed-off-by: Cong Liu <[email protected]>

Looks sane to me, but I'm really not involved enough to fully judge.

Acked-by: Christian König <[email protected]>

> ---
> drivers/gpu/drm/qxl/qxl_kms.c | 4 ++--
> drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 4dc5ad13f12c..a054e4a00fe8 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -165,7 +165,7 @@ int qxl_device_init(struct qxl_device *qdev,
> (int)qdev->surfaceram_size / 1024,
> (sb == 4) ? "64bit" : "32bit");
>
> - qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> + qdev->rom = ioremap_wc(qdev->rom_base, qdev->rom_size);
> if (!qdev->rom) {
> pr_err("Unable to ioremap ROM\n");
> r = -ENOMEM;
> @@ -183,7 +183,7 @@ int qxl_device_init(struct qxl_device *qdev,
> goto rom_unmap;
> }
>
> - qdev->ram_header = ioremap(qdev->vram_base +
> + qdev->ram_header = ioremap_wc(qdev->vram_base +
> qdev->rom->ram_header_offset,
> sizeof(*qdev->ram_header));
> if (!qdev->ram_header) {
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index b2e33d5ba5d0..95df5750f47f 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -82,13 +82,13 @@ int qxl_ttm_io_mem_reserve(struct ttm_device *bdev,
> case TTM_PL_VRAM:
> mem->bus.is_iomem = true;
> mem->bus.offset = (mem->start << PAGE_SHIFT) + qdev->vram_base;
> - mem->bus.caching = ttm_cached;
> + mem->bus.caching = ttm_write_combined;
> break;
> case TTM_PL_PRIV:
> mem->bus.is_iomem = true;
> mem->bus.offset = (mem->start << PAGE_SHIFT) +
> qdev->surfaceram_base;
> - mem->bus.caching = ttm_cached;
> + mem->bus.caching = ttm_write_combined;
> break;
> default:
> return -EINVAL;

2022-03-30 14:39:02

by Cong Liu

Subject: Re: [PATCH v2] drm/qxl: fix qxl can't use in arm64

any suggestions or extra test I can do now?

Regards,
Cong

On 2022/3/25 15:45, Christian König wrote:
> Am 24.03.22 um 11:49 schrieb Cong Liu:
>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>> access. and qxl is a virtual device, it can be treated more like RAM
>> than actual MMIO registers. use ioremap_wc() replace it.
>>
>> Signed-off-by: Cong Liu <[email protected]>
>
> Looks sane to me, but I'm really not involved enough to fully judge.
>
> Acked-by: Christian König <[email protected]>
>
>> ---
>>   drivers/gpu/drm/qxl/qxl_kms.c | 4 ++--
>>   drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
>> b/drivers/gpu/drm/qxl/qxl_kms.c
>> index 4dc5ad13f12c..a054e4a00fe8 100644
>> --- a/drivers/gpu/drm/qxl/qxl_kms.c
>> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
>> @@ -165,7 +165,7 @@ int qxl_device_init(struct qxl_device *qdev,
>>            (int)qdev->surfaceram_size / 1024,
>>            (sb == 4) ? "64bit" : "32bit");
>> -    qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
>> +    qdev->rom = ioremap_wc(qdev->rom_base, qdev->rom_size);
>>       if (!qdev->rom) {
>>           pr_err("Unable to ioremap ROM\n");
>>           r = -ENOMEM;
>> @@ -183,7 +183,7 @@ int qxl_device_init(struct qxl_device *qdev,
>>           goto rom_unmap;
>>       }
>> -    qdev->ram_header = ioremap(qdev->vram_base +
>> +    qdev->ram_header = ioremap_wc(qdev->vram_base +
>>                      qdev->rom->ram_header_offset,
>>                      sizeof(*qdev->ram_header));
>>       if (!qdev->ram_header) {
>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
>> b/drivers/gpu/drm/qxl/qxl_ttm.c
>> index b2e33d5ba5d0..95df5750f47f 100644
>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>> @@ -82,13 +82,13 @@ int qxl_ttm_io_mem_reserve(struct ttm_device *bdev,
>>       case TTM_PL_VRAM:
>>           mem->bus.is_iomem = true;
>>           mem->bus.offset = (mem->start << PAGE_SHIFT) + qdev->vram_base;
>> -        mem->bus.caching = ttm_cached;
>> +        mem->bus.caching = ttm_write_combined;
>>           break;
>>       case TTM_PL_PRIV:
>>           mem->bus.is_iomem = true;
>>           mem->bus.offset = (mem->start << PAGE_SHIFT) +
>>               qdev->surfaceram_base;
>> -        mem->bus.caching = ttm_cached;
>> +        mem->bus.caching = ttm_write_combined;
>>           break;
>>       default:
>>           return -EINVAL;
>

2022-03-31 03:58:29

by David Airlie

Subject: Re: [PATCH v2] drm/qxl: fix qxl can't use in arm64

I'd like to make sure this has no side effects on x86 guests, it
probably is safe, but keep an eye for regression reports.

Reviewed-by: Dave Airlie <[email protected]>
Dave.

On Wed, Mar 30, 2022 at 8:20 PM Cong Liu <[email protected]> wrote:
>
> any suggestions or extra test I can do now?
>
> Regards,
> Cong
>
> On 2022/3/25 15:45, Christian König wrote:
> > Am 24.03.22 um 11:49 schrieb Cong Liu:
> >> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> >> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> >> access. and qxl is a virtual device, it can be treated more like RAM
> >> than actual MMIO registers. use ioremap_wc() replace it.
> >>
> >> Signed-off-by: Cong Liu <[email protected]>
> >
> > Looks sane to me, but I'm really not involved enough to fully judge.
> >
> > Acked-by: Christian König <[email protected]>
> >
> >> ---
> >> drivers/gpu/drm/qxl/qxl_kms.c | 4 ++--
> >> drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
> >> b/drivers/gpu/drm/qxl/qxl_kms.c
> >> index 4dc5ad13f12c..a054e4a00fe8 100644
> >> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> >> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> >> @@ -165,7 +165,7 @@ int qxl_device_init(struct qxl_device *qdev,
> >> (int)qdev->surfaceram_size / 1024,
> >> (sb == 4) ? "64bit" : "32bit");
> >> - qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> >> + qdev->rom = ioremap_wc(qdev->rom_base, qdev->rom_size);
> >> if (!qdev->rom) {
> >> pr_err("Unable to ioremap ROM\n");
> >> r = -ENOMEM;
> >> @@ -183,7 +183,7 @@ int qxl_device_init(struct qxl_device *qdev,
> >> goto rom_unmap;
> >> }
> >> - qdev->ram_header = ioremap(qdev->vram_base +
> >> + qdev->ram_header = ioremap_wc(qdev->vram_base +
> >> qdev->rom->ram_header_offset,
> >> sizeof(*qdev->ram_header));
> >> if (!qdev->ram_header) {
> >> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
> >> b/drivers/gpu/drm/qxl/qxl_ttm.c
> >> index b2e33d5ba5d0..95df5750f47f 100644
> >> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> >> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> >> @@ -82,13 +82,13 @@ int qxl_ttm_io_mem_reserve(struct ttm_device *bdev,
> >> case TTM_PL_VRAM:
> >> mem->bus.is_iomem = true;
> >> mem->bus.offset = (mem->start << PAGE_SHIFT) + qdev->vram_base;
> >> - mem->bus.caching = ttm_cached;
> >> + mem->bus.caching = ttm_write_combined;
> >> break;
> >> case TTM_PL_PRIV:
> >> mem->bus.is_iomem = true;
> >> mem->bus.offset = (mem->start << PAGE_SHIFT) +
> >> qdev->surfaceram_base;
> >> - mem->bus.caching = ttm_cached;
> >> + mem->bus.caching = ttm_write_combined;
> >> break;
> >> default:
> >> return -EINVAL;
> >
>