Skip to content

Conversation

@sukrutb
Copy link

@sukrutb sukrutb commented Dec 13, 2024

Update splash screen details with defconfig changes to enable splash screen.
Add steps to test the splash screen.
Add the reference output image on AM335x EVM.

@praneethbajjuri
Copy link
Collaborator

praneethbajjuri commented Dec 13, 2024

@devarsht , @jainswamil please review

@praneethbajjuri praneethbajjuri force-pushed the sbellary/ti_u-boot_splash_screen_update branch from 074b933 to 0cbc0f5 Compare December 13, 2024 16:21
@praneethbajjuri
Copy link
Collaborator

@sukrutb please update

Copy link
Member

@cshilwant cshilwant left a comment

Choose a reason for hiding this comment

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

Please plan to use ref and cross reference this section under AM335x release notes.

Checkout following links for more details,

How to use ref - https://www.sphinx-doc.org/en/master/usage/referencing.html#cross-referencing-arbitrary-locations

Link to AM335X release notes - https://github.com/TexasInstruments/processor-sdk-doc/blob/master/source/devices/AM335X/linux/Release_Specific_Release_Notes.rst?plain=1#L60

Copy link
Collaborator

@devarsht devarsht left a comment

Choose a reason for hiding this comment

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

@sukrutb @praneethbajjuri

Ain't it more apt to use UG-Splash screen i.e. below instead of APP-Splash-screen, For all Sitara platforms we use UG-Splash screen so that guide is more evolved.
https://github.com/TexasInstruments/processor-sdk-doc/blob/0cbc0f59f3aea10940952042012043c21c232fc7/source/linux/Foundational_Components/U-Boot/UG-Splash-Screen.rst

@sukrutb
Copy link
Author

sukrutb commented Dec 17, 2024

@sukrutb @praneethbajjuri

Ain't it more apt to use UG-Splash screen i.e. below instead of APP-Splash-screen, For all Sitara platforms we use UG-Splash screen so that guide is more evolved. https://github.com/TexasInstruments/processor-sdk-doc/blob/0cbc0f59f3aea10940952042012043c21c232fc7/source/linux/Foundational_Components/U-Boot/UG-Splash-Screen.rst
@devarsht I am not familiar with the cross platform reference of the release notes. Can you please elaborate?
AFAIK, this is related to AM62x (specifically HW, u-boot SW, and supported features), which is not the same as AM33xx implementation(e.g., we don't have SPL splash screen support).
So, I have updated in the APP-Splash-screen.

Copy link
Member

@cshilwant cshilwant left a comment

Choose a reason for hiding this comment

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

@sukrutb I still don't see this as well as other references of code section indentation under code-blocks being fixed - https://github.com/TexasInstruments/processor-sdk-doc/pull/116/files#r1887764585

@devarsht
Copy link
Collaborator

@sukrutb @praneethbajjuri
Ain't it more apt to use UG-Splash screen i.e. below instead of APP-Splash-screen, For all Sitara platforms we use UG-Splash screen so that guide is more evolved. https://github.com/TexasInstruments/processor-sdk-doc/blob/0cbc0f59f3aea10940952042012043c21c232fc7/source/linux/Foundational_Components/U-Boot/UG-Splash-Screen.rst
@devarsht I am not familiar with the cross platform reference of the release notes. Can you please elaborate?
AFAIK, this is related to AM62x (specifically HW, u-boot SW, and supported features), which is not the same as AM33xx implementation(e.g., we don't have SPL splash screen support).
So, I have updated in the APP-Splash-screen.

The UG-Splash-screen.rst is not specific to AM62x, other SoCs like AM62p too use the same guide. I am not sure about the goal for app-splash-screen.rst need to check with Bin on this.

Copy link
Collaborator

@devarsht devarsht left a comment

Choose a reason for hiding this comment

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

@sukrutb , anything to add from perspective of flicker-free transition solution ? I remember you did some enablement using pwm preservation and some discussions were there with simple-framebuffer too ?

@cshilwant
Copy link
Member

@sukrutb I see many unrelated commits. Can you rebase and ensure only intended commits are in your PR

@sukrutb sukrutb force-pushed the sbellary/ti_u-boot_splash_screen_update branch 2 times, most recently from 3a25f5e to 73b6f6e Compare December 20, 2024 19:13
@sukrutb
Copy link
Author

sukrutb commented Dec 20, 2024

@sukrutb I see many unrelated commits. Can you rebase and ensure only intended commits are in your PR

@cshilwant , Done.

@cshilwant cshilwant dismissed stale reviews from StaticRocket and themself December 22, 2024 11:50

Author has updated the PR

Copy link
Member

@cshilwant cshilwant left a comment

Choose a reason for hiding this comment

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

@sukrutb I still see 5 commits on this PR.

I believe you used the Github's apply suggestion feature?

Apparently that's not encouraged. Author's are expected to resolve comments from reviewers manually.

The Github's apply suggestion feature messes with the Signed-off-by tags

FYR, I see below in 1286639

Signed-off-by: Sukrut Bellary <[email protected]>

Update the splash screen details with defconfig changes to enable
the splash screen with backlight.
Add steps to test the splash screen.
Add the reference output image on AM335x EVM.

Signed-off-by: Sukrut Bellary <[email protected]>
@sukrutb sukrutb force-pushed the sbellary/ti_u-boot_splash_screen_update branch from 73b6f6e to 8d9b282 Compare December 23, 2024 22:54
@sukrutb
Copy link
Author

sukrutb commented Dec 23, 2024

@sukrutb I still see 5 commits on this PR.

I believe you used the Github's apply suggestion feature?

Apparently that's not encouraged. Author's are expected to resolve comments from reviewers manually.

The Github's apply suggestion feature messes with the Signed-off-by tags

FYR, I see below in 1286639

Signed-off-by: Sukrut Bellary <[email protected]>

@cshilwant , fixed. Please review. Thanks.

@sukrutb sukrutb requested a review from cshilwant December 23, 2024 23:38
@cshilwant
Copy link
Member

@praneethbajjuri @StaticRocket @devarsht for re-review

@praneethbajjuri praneethbajjuri merged commit 5159030 into TexasInstruments:master Dec 25, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants