diff mbox

[00/48] Etnaviv changes RFCv1->RFCv2

Message ID CAH9NwWehyAtECA=uWn1TuEo6N9WwHXMtsBRoxTz7WvvdnrN3Hw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christian Gmeiner Sept. 30, 2015, 7:53 a.m. UTC
Hi Lucas,

2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> Hi Christian,
>
> Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
>> Hi Lucas.
>>
>> I think I have run into a cache flush / cache coherency issue. I will
>> try to reproduce this issue with a small example and will
>> keep you updated.
>
> What are the symptoms of the issue you are hitting? Maybe I can
> reproduce or see if I have an idea right away.
>

With the help of the etnaviv_2d_test in my libdrm repo on github I was able
to test different bo flags.

ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
The rendering result looks as expected. If I try ETNA_BO_WC the rendering
result looks different for every run.

debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
Version: 1.0.0
  Name: etnaviv
  Date: 20150910
  Description: etnaviv DRM
bo cpu prep: 0
debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
052880d433e1bf495e268206addd4087  /tmp/etna.bmp
debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
Version: 1.0.0
  Name: etnaviv
  Date: 20150910
  Description: etnaviv DRM
bo cpu prep: 0
debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
Version: 1.0.0
  Name: etnaviv
  Date: 20150910
  Description: etnaviv DRM
bo cpu prep: 0
debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp

etna_cmd_stream_finish() waits until the submitted command stream was
processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
help.

I am doing something wrong? Should this work in theory?


Greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner

Comments

Lucas Stach Oct. 1, 2015, 8:50 a.m. UTC | #1
Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
> Hi Lucas,
> 
> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> > Hi Christian,
> >
> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
> >> Hi Lucas.
> >>
> >> I think I have run into a cache flush / cache coherency issue. I will
> >> try to reproduce this issue with a small example and will
> >> keep you updated.
> >
> > What are the symptoms of the issue you are hitting? Maybe I can
> > reproduce or see if I have an idea right away.
> >
> 
> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
> to test different bo flags.
> 
> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
> result looks different for every run.
> 
I have an idea what's going wrong here. Will keep you updated.

Regards,
Lucas

> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
> 
> etna_cmd_stream_finish() waits until the submitted command stream was
> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
> help.
> 
> I am doing something wrong? Should this work in theory?
> 
> diff --git a/tests/etnaviv/etnaviv_2d_test.c b/tests/etnaviv/etnaviv_2d_test.c
> index e1ee8a8..037da5b 100644
> --- a/tests/etnaviv/etnaviv_2d_test.c
> +++ b/tests/etnaviv/etnaviv_2d_test.c
> @@ -200,7 +200,7 @@ int main(int argc, char *argv[])
>                 goto fail;
>         }
> 
> -       bmp = etna_bo_new(dev, bmp_size, ETNA_BO_UNCACHED);
> +       bmp = etna_bo_new(dev, bmp_size, ETNA_BO_WC);
>         if (!bmp) {
>                 ret = 5;
>                 goto fail;
> @@ -218,8 +218,10 @@ int main(int argc, char *argv[])
> 
>         etna_cmd_stream_finish(stream);
> 
> +       int state = etna_bo_cpu_prep(bmp, DRM_ETNA_PREP_READ);
> +       printf("bo cpu prep: %d\n", state);
>         bmp_dump32(etna_bo_map(bmp), width, height, false, "/tmp/etna.bmp");
> -
> +       etna_bo_cpu_fini(bmp);
>  fail:
>         if (stream)
>                 etna_cmd_stream_del(stream);
> 
> Greets
> --
> Christian Gmeiner, MSc
> 
> https://soundcloud.com/christian-gmeiner
Lucas Stach Oct. 13, 2015, 8:25 a.m. UTC | #2
Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
> Hi Lucas,
> 
> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> > Hi Christian,
> >
> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
> >> Hi Lucas.
> >>
> >> I think I have run into a cache flush / cache coherency issue. I will
> >> try to reproduce this issue with a small example and will
> >> keep you updated.
> >
> > What are the symptoms of the issue you are hitting? Maybe I can
> > reproduce or see if I have an idea right away.
> >
> 
> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
> to test different bo flags.
> 
> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
> result looks different for every run.
> 
> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> Version: 1.0.0
>   Name: etnaviv
>   Date: 20150910
>   Description: etnaviv DRM
> bo cpu prep: 0
> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
> 
> etna_cmd_stream_finish() waits until the submitted command stream was
> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
> help.
> 
> I am doing something wrong? Should this work in theory?
> 
For the record: this is caused by the "shared attribute override enable"
bit in the AUX_CTRL register of the PL310 cache controller not being
set, which makes it non-compliant with the ARMv7 ARM specified behavior
of conflicting memory aliases.
The etnaviv kernel driver makes sure to clean the cachable alias before
handing out the pages, but without this bit set the L2 controller will
turn the userspace bufferable reads into "cachable, no allocate" which
will lead to userspace hitting stale, non-evicted cache lines.

This can be worked around by adding a "arm,shared-override" property to
the L2 cache DT node. This will only work if the kernel is booted in
secure state and will fault on a NS kernel. So this should be considered
a hack and the bootloader/firmware should make sure to set this bit.

Regards,
Lucas
Christian Gmeiner Oct. 20, 2015, 7:20 a.m. UTC | #3
Hi Lucas,

2015-10-13 10:25 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
>> Hi Lucas,
>>
>> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>> > Hi Christian,
>> >
>> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
>> >> Hi Lucas.
>> >>
>> >> I think I have run into a cache flush / cache coherency issue. I will
>> >> try to reproduce this issue with a small example and will
>> >> keep you updated.
>> >
>> > What are the symptoms of the issue you are hitting? Maybe I can
>> > reproduce or see if I have an idea right away.
>> >
>>
>> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
>> to test different bo flags.
>>
>> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
>> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
>> result looks different for every run.
>>
>> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> Version: 1.0.0
>>   Name: etnaviv
>>   Date: 20150910
>>   Description: etnaviv DRM
>> bo cpu prep: 0
>> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
>> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> Version: 1.0.0
>>   Name: etnaviv
>>   Date: 20150910
>>   Description: etnaviv DRM
>> bo cpu prep: 0
>> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
>> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> Version: 1.0.0
>>   Name: etnaviv
>>   Date: 20150910
>>   Description: etnaviv DRM
>> bo cpu prep: 0
>> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
>>
>> etna_cmd_stream_finish() waits until the submitted command stream was
>> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
>> help.
>>
>> I am doing something wrong? Should this work in theory?
>>
> For the record: this is caused by the "shared attribute override enable"
> bit in the AUX_CTRL register of the PL310 cache controller not being
> set, which makes it non-compliant with the ARMv7 ARM specified behavior
> of conflicting memory aliases.
> The etnaviv kernel driver makes sure to clean the cachable alias before
> handing out the pages, but without this bit set the L2 controller will
> turn the userspace bufferable reads into "cachable, no allocate" which
> will lead to userspace hitting stale, non-evicted cache lines.
>
> This can be worked around by adding a "arm,shared-override" property to
> the L2 cache DT node. This will only work if the kernel is booted in
> secure state and will fault on a NS kernel. So this should be considered
> a hack and the bootloader/firmware should make sure to set this bit.
>

Is the kernel able to detect this faulty environment? It would be great
if we can warn the user about this issue and 'convert' all ETNA_BO_WC
request to ETNA_BO_CACHED or ETNA_BO_UNCACHED.

Else there might be some bug reports in the future where something is
broken due to a bad environment and these kind of bugs are hard to
sort out.

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner
Lucas Stach Oct. 20, 2015, 9 a.m. UTC | #4
Am Dienstag, den 20.10.2015, 09:20 +0200 schrieb Christian Gmeiner:
> Hi Lucas,
> 
> 2015-10-13 10:25 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> > Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
> >> Hi Lucas,
> >>
> >> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> >> > Hi Christian,
> >> >
> >> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
> >> >> Hi Lucas.
> >> >>
> >> >> I think I have run into a cache flush / cache coherency issue. I will
> >> >> try to reproduce this issue with a small example and will
> >> >> keep you updated.
> >> >
> >> > What are the symptoms of the issue you are hitting? Maybe I can
> >> > reproduce or see if I have an idea right away.
> >> >
> >>
> >> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
> >> to test different bo flags.
> >>
> >> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
> >> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
> >> result looks different for every run.
> >>
> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> >> Version: 1.0.0
> >>   Name: etnaviv
> >>   Date: 20150910
> >>   Description: etnaviv DRM
> >> bo cpu prep: 0
> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> >> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> >> Version: 1.0.0
> >>   Name: etnaviv
> >>   Date: 20150910
> >>   Description: etnaviv DRM
> >> bo cpu prep: 0
> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> >> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
> >> Version: 1.0.0
> >>   Name: etnaviv
> >>   Date: 20150910
> >>   Description: etnaviv DRM
> >> bo cpu prep: 0
> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
> >> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
> >>
> >> etna_cmd_stream_finish() waits until the submitted command stream was
> >> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
> >> help.
> >>
> >> I am doing something wrong? Should this work in theory?
> >>
> > For the record: this is caused by the "shared attribute override enable"
> > bit in the AUX_CTRL register of the PL310 cache controller not being
> > set, which makes it non-compliant with the ARMv7 ARM specified behavior
> > of conflicting memory aliases.
> > The etnaviv kernel driver makes sure to clean the cachable alias before
> > handing out the pages, but without this bit set the L2 controller will
> > turn the userspace bufferable reads into "cachable, no allocate" which
> > will lead to userspace hitting stale, non-evicted cache lines.
> >
> > This can be worked around by adding a "arm,shared-override" property to
> > the L2 cache DT node. This will only work if the kernel is booted in
> > secure state and will fault on a NS kernel. So this should be considered
> > a hack and the bootloader/firmware should make sure to set this bit.
> >
> 
> Is the kernel able to detect this faulty environment? It would be great
> if we can warn the user about this issue and 'convert' all ETNA_BO_WC
> request to ETNA_BO_CACHED or ETNA_BO_UNCACHED.
> 
> Else there might be some bug reports in the future where something is
> broken due to a bad environment and these kind of bugs are hard to
> sort out.
> 
I don't think we should work around a platform issue in individual
drivers. I mean etnaviv makes the issue really visible, but not having
this bit set in the PL310 controller makes the whole platform
non-conforming to what is specified in the ARMv7 ARM, so the platform
may exhibit all kinds of subtle breakages anyway.

We may check for this bit in the PL310 initialization and warn the user
that a firmware update might be needed to fix this. This should be
enough to sort out bug reports caused by this specific issue.

Regards,
Lucas
Jon Nettleton Oct. 20, 2015, 9:09 a.m. UTC | #5
On Tue, Oct 20, 2015 at 11:00 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 20.10.2015, 09:20 +0200 schrieb Christian Gmeiner:
>> Hi Lucas,
>>
>> 2015-10-13 10:25 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>> > Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
>> >> Hi Lucas,
>> >>
>> >> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>> >> > Hi Christian,
>> >> >
>> >> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
>> >> >> Hi Lucas.
>> >> >>
>> >> >> I think I have run into a cache flush / cache coherency issue. I will
>> >> >> try to reproduce this issue with a small example and will
>> >> >> keep you updated.
>> >> >
>> >> > What are the symptoms of the issue you are hitting? Maybe I can
>> >> > reproduce or see if I have an idea right away.
>> >> >
>> >>
>> >> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
>> >> to test different bo flags.
>> >>
>> >> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
>> >> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
>> >> result looks different for every run.
>> >>
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
>> >>
>> >> etna_cmd_stream_finish() waits until the submitted command stream was
>> >> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
>> >> help.
>> >>
>> >> I am doing something wrong? Should this work in theory?
>> >>
>> > For the record: this is caused by the "shared attribute override enable"
>> > bit in the AUX_CTRL register of the PL310 cache controller not being
>> > set, which makes it non-compliant with the ARMv7 ARM specified behavior
>> > of conflicting memory aliases.
>> > The etnaviv kernel driver makes sure to clean the cachable alias before
>> > handing out the pages, but without this bit set the L2 controller will
>> > turn the userspace bufferable reads into "cachable, no allocate" which
>> > will lead to userspace hitting stale, non-evicted cache lines.
>> >
>> > This can be worked around by adding a "arm,shared-override" property to
>> > the L2 cache DT node. This will only work if the kernel is booted in
>> > secure state and will fault on a NS kernel. So this should be considered
>> > a hack and the bootloader/firmware should make sure to set this bit.
>> >
>>
>> Is the kernel able to detect this faulty environment? It would be great
>> if we can warn the user about this issue and 'convert' all ETNA_BO_WC
>> request to ETNA_BO_CACHED or ETNA_BO_UNCACHED.
>>
>> Else there might be some bug reports in the future where something is
>> broken due to a bad environment and these kind of bugs are hard to
>> sort out.
>>
> I don't think we should work around a platform issue in individual
> drivers. I mean etnaviv makes the issue really visible, but not having
> this bit set in the PL310 controller makes the whole platform
> non-conforming to what is specified in the ARMv7 ARM, so the platform
> may exhibit all kinds of subtle breakages anyway.
>
> We may check for this bit in the PL310 initialization and warn the user
> that a firmware update might be needed to fix this. This should be
> enough to sort out bug reports caused by this specific issue.
>

I agree, although we can also point them to the device-tree option to
workaround the problem temporarily.

Christian, fyi I have fixed this in u-boot for all the SolidRun platforms.
Christian Gmeiner Oct. 20, 2015, 9:39 a.m. UTC | #6
Hi Lucas,


2015-10-20 11:00 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> Am Dienstag, den 20.10.2015, 09:20 +0200 schrieb Christian Gmeiner:
>> Hi Lucas,
>>
>> 2015-10-13 10:25 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>> > Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
>> >> Hi Lucas,
>> >>
>> >> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>> >> > Hi Christian,
>> >> >
>> >> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
>> >> >> Hi Lucas.
>> >> >>
>> >> >> I think I have run into a cache flush / cache coherency issue. I will
>> >> >> try to reproduce this issue with a small example and will
>> >> >> keep you updated.
>> >> >
>> >> > What are the symptoms of the issue you are hitting? Maybe I can
>> >> > reproduce or see if I have an idea right away.
>> >> >
>> >>
>> >> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
>> >> to test different bo flags.
>> >>
>> >> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
>> >> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
>> >> result looks different for every run.
>> >>
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>> >> Version: 1.0.0
>> >>   Name: etnaviv
>> >>   Date: 20150910
>> >>   Description: etnaviv DRM
>> >> bo cpu prep: 0
>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>> >> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
>> >>
>> >> etna_cmd_stream_finish() waits until the submitted command stream was
>> >> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
>> >> help.
>> >>
>> >> I am doing something wrong? Should this work in theory?
>> >>
>> > For the record: this is caused by the "shared attribute override enable"
>> > bit in the AUX_CTRL register of the PL310 cache controller not being
>> > set, which makes it non-compliant with the ARMv7 ARM specified behavior
>> > of conflicting memory aliases.
>> > The etnaviv kernel driver makes sure to clean the cachable alias before
>> > handing out the pages, but without this bit set the L2 controller will
>> > turn the userspace bufferable reads into "cachable, no allocate" which
>> > will lead to userspace hitting stale, non-evicted cache lines.
>> >
>> > This can be worked around by adding a "arm,shared-override" property to
>> > the L2 cache DT node. This will only work if the kernel is booted in
>> > secure state and will fault on a NS kernel. So this should be considered
>> > a hack and the bootloader/firmware should make sure to set this bit.
>> >
>>
>> Is the kernel able to detect this faulty environment? It would be great
>> if we can warn the user about this issue and 'convert' all ETNA_BO_WC
>> request to ETNA_BO_CACHED or ETNA_BO_UNCACHED.
>>
>> Else there might be some bug reports in the future where something is
>> broken due to a bad environment and these kind of bugs are hard to
>> sort out.
>>
> I don't think we should work around a platform issue in individual
> drivers. I mean etnaviv makes the issue really visible, but not having
> this bit set in the PL310 controller makes the whole platform
> non-conforming to what is specified in the ARMv7 ARM, so the platform
> may exhibit all kinds of subtle breakages anyway.
>

I am fine with that.

> We may check for this bit in the PL310 initialization and warn the user
> that a firmware update might be needed to fix this. This should be
> enough to sort out bug reports caused by this specific issue.
>

That is all I want. If you have a patch for that put me on CC and I can
test/review it.

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner
Christian Gmeiner Oct. 20, 2015, 9:40 a.m. UTC | #7
Hi Jon,

2015-10-20 11:09 GMT+02:00 Jon Nettleton <jon.nettleton@gmail.com>:
> On Tue, Oct 20, 2015 at 11:00 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>> Am Dienstag, den 20.10.2015, 09:20 +0200 schrieb Christian Gmeiner:
>>> Hi Lucas,
>>>
>>> 2015-10-13 10:25 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>>> > Am Mittwoch, den 30.09.2015, 09:53 +0200 schrieb Christian Gmeiner:
>>> >> Hi Lucas,
>>> >>
>>> >> 2015-09-28 12:39 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
>>> >> > Hi Christian,
>>> >> >
>>> >> > Am Montag, den 28.09.2015, 11:46 +0200 schrieb Christian Gmeiner:
>>> >> >> Hi Lucas.
>>> >> >>
>>> >> >> I think I have run into a cache flush / cache coherency issue. I will
>>> >> >> try to reproduce this issue with a small example and will
>>> >> >> keep you updated.
>>> >> >
>>> >> > What are the symptoms of the issue you are hitting? Maybe I can
>>> >> > reproduce or see if I have an idea right away.
>>> >> >
>>> >>
>>> >> With the help of the etnaviv_2d_test in my libdrm repo on github I was able
>>> >> to test different bo flags.
>>> >>
>>> >> ETNA_BO_UNCACHED and ETNA_BO_CACHED are working as expected.
>>> >> The rendering result looks as expected. If I try ETNA_BO_WC the rendering
>>> >> result looks different for every run.
>>> >>
>>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>>> >> Version: 1.0.0
>>> >>   Name: etnaviv
>>> >>   Date: 20150910
>>> >>   Description: etnaviv DRM
>>> >> bo cpu prep: 0
>>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>>> >> 052880d433e1bf495e268206addd4087  /tmp/etna.bmp
>>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>>> >> Version: 1.0.0
>>> >>   Name: etnaviv
>>> >>   Date: 20150910
>>> >>   Description: etnaviv DRM
>>> >> bo cpu prep: 0
>>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>>> >> f1a02a52d81c0b79b098877e6b7d9303  /tmp/etna.bmp
>>> >> debian@cubox:~/libdrm$ tests/etnaviv/etnaviv_2d_test /dev/dri/card1
>>> >> Version: 1.0.0
>>> >>   Name: etnaviv
>>> >>   Date: 20150910
>>> >>   Description: etnaviv DRM
>>> >> bo cpu prep: 0
>>> >> debian@cubox:~/libdrm$ md5sum /tmp/etna.bmp
>>> >> de5a428eb1f6567849ef40a944a995b8  /tmp/etna.bmp
>>> >>
>>> >> etna_cmd_stream_finish() waits until the submitted command stream was
>>> >> processed by the GPU. I tried to use etna_bo_cpu_prep(..) but I that did not
>>> >> help.
>>> >>
>>> >> I am doing something wrong? Should this work in theory?
>>> >>
>>> > For the record: this is caused by the "shared attribute override enable"
>>> > bit in the AUX_CTRL register of the PL310 cache controller not being
>>> > set, which makes it non-compliant with the ARMv7 ARM specified behavior
>>> > of conflicting memory aliases.
>>> > The etnaviv kernel driver makes sure to clean the cachable alias before
>>> > handing out the pages, but without this bit set the L2 controller will
>>> > turn the userspace bufferable reads into "cachable, no allocate" which
>>> > will lead to userspace hitting stale, non-evicted cache lines.
>>> >
>>> > This can be worked around by adding a "arm,shared-override" property to
>>> > the L2 cache DT node. This will only work if the kernel is booted in
>>> > secure state and will fault on a NS kernel. So this should be considered
>>> > a hack and the bootloader/firmware should make sure to set this bit.
>>> >
>>>
>>> Is the kernel able to detect this faulty environment? It would be great
>>> if we can warn the user about this issue and 'convert' all ETNA_BO_WC
>>> request to ETNA_BO_CACHED or ETNA_BO_UNCACHED.
>>>
>>> Else there might be some bug reports in the future where something is
>>> broken due to a bad environment and these kind of bugs are hard to
>>> sort out.
>>>
>> I don't think we should work around a platform issue in individual
>> drivers. I mean etnaviv makes the issue really visible, but not having
>> this bit set in the PL310 controller makes the whole platform
>> non-conforming to what is specified in the ARMv7 ARM, so the platform
>> may exhibit all kinds of subtle breakages anyway.
>>
>> We may check for this bit in the PL310 initialization and warn the user
>> that a firmware update might be needed to fix this. This should be
>> enough to sort out bug reports caused by this specific issue.
>>
>
> I agree, although we can also point them to the device-tree option to
> workaround the problem temporarily.
>
> Christian, fyi I have fixed this in u-boot for all the SolidRun platforms.

Great - will update all my devices.

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner
Fabio Estevam Oct. 20, 2015, 10:42 a.m. UTC | #8
Hi Jon,

On Tue, Oct 20, 2015 at 7:09 AM, Jon Nettleton <jon.nettleton@gmail.com> wrote:

> I agree, although we can also point them to the device-tree option to
> workaround the problem temporarily.
>
> Christian, fyi I have fixed this in u-boot for all the SolidRun platforms.

This is fixed in mainline U-boot for mx6:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=b4ed9f86df441d7ac304f70cc711c257e7c8ebf1;hp=9927d60f4aadcecbd3143400d01ad4500438ea4f
diff mbox

Patch

diff --git a/tests/etnaviv/etnaviv_2d_test.c b/tests/etnaviv/etnaviv_2d_test.c
index e1ee8a8..037da5b 100644
--- a/tests/etnaviv/etnaviv_2d_test.c
+++ b/tests/etnaviv/etnaviv_2d_test.c
@@ -200,7 +200,7 @@  int main(int argc, char *argv[])
                goto fail;
        }

-       bmp = etna_bo_new(dev, bmp_size, ETNA_BO_UNCACHED);
+       bmp = etna_bo_new(dev, bmp_size, ETNA_BO_WC);
        if (!bmp) {
                ret = 5;
                goto fail;
@@ -218,8 +218,10 @@  int main(int argc, char *argv[])

        etna_cmd_stream_finish(stream);

+       int state = etna_bo_cpu_prep(bmp, DRM_ETNA_PREP_READ);
+       printf("bo cpu prep: %d\n", state);
        bmp_dump32(etna_bo_map(bmp), width, height, false, "/tmp/etna.bmp");
-
+       etna_bo_cpu_fini(bmp);
 fail:
        if (stream)
                etna_cmd_stream_del(stream);