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

[DNM] [experimental-tmp] Add LG Nexus 5X support #220

Open
wants to merge 2 commits into
base: experimental-tmp
Choose a base branch
from

Conversation

Dominduchami
Copy link

Add support for LG Nexus 5X. Also enable autorefresh so that the framebuffer works on linux.

I would greatly appriciate any feedback regarding the framebuffer enabler, the way this is done here feels really hacky.

@@ -40,6 +40,9 @@ static void mdp_cmd_signal_refresh(void)

static void mdp_cmd_refresh_start(struct fbcon_config *fb)
{
#if ENABLE_AUTOREFRESH
Copy link
Member

Choose a reason for hiding this comment

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

The reason hw autorefresh wasn't used is because it didn't work on some platforms (i.e. on 8974) so it was decided to use a more "reliable" (even if ugly) solution. See #137 that adds this thread as well as a manual autorefresh kick via fastboot.

Copy link
Member

Choose a reason for hiding this comment

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

I want to avoid making such changes unconditionally. lk2nd is meant to boot a variety of different boot images (downstream, mainline, lk2nd itself, other operating systems) and enabling autorefresh may break booting them (downstream would be the most likely candidate for that).

However, we could potentially enable autorefresh immediately before booting a Linux that needs this. It could be either detected using a special parameter on the kernel command line (e.g. lk2nd.display-autorefresh). Or we could introduce special magic: If /chosen/simple-framebuffer exists then enable autorefresh.

Choose a reason for hiding this comment

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

Appreciate any help at all thx

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

I want to avoid making such changes unconditionally. lk2nd is meant to boot a variety of different boot images (downstream, mainline, lk2nd itself, other operating systems) and enabling autorefresh may break booting them (downstream would be the most likely candidate for that).

Ah, I understand, makes sense :-) Downstream probably wouldn't like that.

However, we could potentially enable autorefresh immediately before booting a Linux that needs this. It could be either detected using a special parameter on the kernel command line (e.g. lk2nd.display-autorefresh). Or we could introduce special magic: If /chosen/simple-framebuffer exists then enable autorefresh.

To me the second option seems better, in that case adding framebuffer on such devices upstream would be the same as normally.

Copy link

Choose a reason for hiding this comment

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

But /chosen/simple-framebuffer would require mainline accepting this, but kernel command line can be used on vanilla mainline (unmodified kernel sources in case mainline does not want to accept /chosen/simple-framebuffer.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I was looking at provisioning the full framebuffer data from the lk2nd, which could include enabling autorefresh as one of the steps. https://github.com/msm8916-mainline/lk2nd/blob/rebase/fb/lk2nd/display/simplefb.c

Interestingly upstream allows extending the simplefb bindings just for the bootloader data so theoretically we could even have custom lk2nd,simple-framebuffer compatible that would allow lk2nd,auto-refresh; property in mainline

Copy link

Choose a reason for hiding this comment

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

Interesting, that would be then good try to send a patch :). I suppose we'd have to add our lk2nd and it's property into allOf: section in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/simple-framebuffer.yaml, right?
BTW what from that file states simplefb bindings can be extended for bootloader data? That example with pipeline for allwinner and amlogic?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, specifically the last part of the long description says firmware may need extra data.

However I think we shouldn't hurry extending it in case lk2nd can handle quirks internally, maybe it's nicer to have the required flags in lk2nd's own dts.


/ {
model = "LG Nexus 5X";
qcom,msm-id = <251 0>, <252 0>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
qcom,msm-id = <251 0>, <252 0>;
qcom,msm-id = <QCOM_ID_MSM8992 0>;

Let's use the qcom,ids.h like all other files around, I also doubt there is a phone with APQ8092 :)

Copy link
Author

Choose a reason for hiding this comment

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

True :-) Sorry, I should've checked other examples before adding that, will fix it once I get home

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants