Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i.MX RT1050: Reactivate data cache #10314

Merged
merged 2 commits into from Apr 18, 2019

Conversation

Projects
None yet
5 participants
@kjbracey-arm
Copy link
Contributor

commented Apr 4, 2019

Description

Since commit 12c6b1b, the i.MX RT1050 has effectively had its data cache disabled, as the SDRAM was marked Shareable; for the Cortex-M7, shareable memory is not cached.

This was done to make the Ethernet driver work without any cache maintenance code. This commit adds cache maintenance and memory barriers to the Ethernet driver, and removes the Shareable attribute from the SDRAM, so the data cache is used again.

Cache code in the base fsl_enet.c driver has not been activated - the bulk of it is in higher-level Read and Write calls that we're not using, and there is one flawed invalidate in its initialisation. Instead imx_emac.cpp takes full cache responsibility.

This commit also marks the SDRAM as read/write-allocate. As the Cortex-M7 has its "Dynamic read allocate mode" to automatically switch back to read-allocate in cases where write allocate is working poorly (eg large memset), this should result in a performance boost with no downside.

Activating write-allocate is also an attempt to provoke any flaws in cache maintenance - the Ethernet transmit buffers for example will be more likely to have a little data in the cache that needs cleaning.

Incorporates and depends on #10299.

Pull request type

[ ] Fix
[ ] Refactor
[X] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@mmahadevan108, @cyborg71

MIMXRT1050: Fix ENET issues
This is a fix for Issue 10239. The change aligns
the receive buffer lengths

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
@kjbracey-arm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

This is me working by inspection - I don't have a board to work with. I'd appreciate confirmation that this (a) works and (b) shows some sort of performance improvement.

@ciarmcom ciarmcom requested review from maclobdell, MarceloSalazar and ARMmbed/mbed-os-maintainers Apr 4, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

i.MX RT1050: Reactivate data cache
Since commit 12c6b1b, the i.MX RT1050 has effectively had its data
cache disabled, as the SDRAM was marked Shareable; for the Cortex-M7,
shareable memory is not cached.

This was done to make the Ethernet driver work without any cache
maintenance code. This commit adds cache maintenance and memory barriers
to the Ethernet driver, and removes the Shareable attribute from the
SDRAM, so the data cache is used again.

Cache code in the base fsl_enet.c driver has not been activated - the
bulk of it is in higher-level Read and Write calls that we're not using,
and there is one flawed invalidate in its initialisation. Instead
imx_emac.cpp takes full cache responsibility.

This commit also marks the SDRAM as read/write-allocate. As the
Cortex-M7 has its "Dynamic read allocate mode" to automatically switch
back to read-allocate in cases where write allocate is working poorly
(eg large memset), this should result in a performance boost with no
downside.

Activating write-allocate is also an attempt to provoke any flaws in
cache maintenance - the Ethernet transmit buffers for example will be
more likely to have a little data in the cache that needs cleaning.

@kjbracey-arm kjbracey-arm force-pushed the kjbracey-arm:rt1050_dcache branch from 497c5f6 to 6fe5076 Apr 4, 2019

@0xc0170 0xc0170 requested a review from mmahadevan108 Apr 4, 2019

@mmahadevan108

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

@kjbracey-arm Thank you for your changes, this works fine with the test provided by @cyborg71. I am running the full suite of mbed-os tests.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

@kjbracey-arm Thank you for your changes, this works fine with the test provided by @cyborg71. I am running the full suite of mbed-os tests.

@mmahadevan108 What are the results? Can you please review this PR

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 17, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test
@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Known failure , not related, will restart tests a bit later once some 5.12.2 PRs progress

@mmahadevan108 All tests passed?

@mmahadevan108

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

The below is not related to this change as I am seeing this on the master branch as well.
I am seeing a strange behavior with test "mbed-os-tests-mbed_hal-sleep_manager_racecondition" when running this with IAR toolchain.
The test passes, however it causes an issue running subsequent tests. I have to power off and on the board for them to run. Any idea why?

@mmahadevan108
Copy link
Contributor

left a comment

@kjbracey-arm Thank you for the change

@kjbracey-arm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

That's... not a very interesting test. Something about what it leaves in RAM confuses the system after reboot?

Is it reliably just that test? What if you just shuffle things around a bit by adding some dummy data in ROM or RAM?

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

CI test restarted

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 18, 2019

@0xc0170 0xc0170 merged commit 3ec9c19 into ARMmbed:master Apr 18, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage Success
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9981 cycles (-352 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@kjbracey-arm kjbracey-arm deleted the kjbracey-arm:rt1050_dcache branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.