Skip to content

i.MX RT1050: Reactivate data cache #10314

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

Merged
merged 2 commits into from
Apr 18, 2019
Merged

Conversation

kjbracey
Copy link
Contributor

@kjbracey kjbracey 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

This is a fix for Issue 10239. The change aligns
the receive buffer lengths

Signed-off-by: Mahesh Mahadevan <[email protected]>
@kjbracey
Copy link
Contributor Author

kjbracey 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 a team April 4, 2019 09:00
@ciarmcom
Copy link
Member

ciarmcom commented Apr 4, 2019

@kjbracey-arm, thank you for your changes.
@maclobdell @MarceloSalazar @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

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.
@mmahadevan108
Copy link
Contributor

@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
Copy link
Contributor

0xc0170 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
Copy link

mbed-ci 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
Copy link
Contributor

0xc0170 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
Copy link
Contributor

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?

Copy link
Contributor

@mmahadevan108 mmahadevan108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kjbracey-arm Thank you for the change

@kjbracey
Copy link
Contributor Author

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
Copy link
Contributor

0xc0170 commented Apr 18, 2019

CI test restarted

@0xc0170 0xc0170 merged commit 3ec9c19 into ARMmbed:master Apr 18, 2019
@kjbracey kjbracey deleted the rt1050_dcache branch May 3, 2019 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants