buzzmarshall has quit [Quit: Konversation terminated!]
hexdump0815 has quit [Ping timeout: 268 seconds]
hexdump0815 has joined #linux-amlogic
naoki has joined #linux-amlogic
djrscally has joined #linux-amlogic
djrscally has quit [Client Quit]
ldevulder has joined #linux-amlogic
naoki has quit [Remote host closed the connection]
naoki has joined #linux-amlogic
<jbrunet>
You could provide the GPIO to a regulator with the appropriate `regulator-enable-ramp-delay` instead of providing it to simple-amplifier.
<jbrunet>
Then provide the regulator as VCC to simple amplifier, I suppose that would do the trick
<jbrunet>
to be tested
<marc|gonzalez>
mripard: I didn't understand your comment to my TDP158 patch ("And since we would need a separate binding file, we probably don't want to add things to simple-bridge that would not be part of its binding")
<marc|gonzalez>
In the context of my proposed change, I don't see what the binding plays in.
<mripard>
you need a different binding
<mripard>
so you wouldn't document something in the simple-bridge binding that isn't going to be used by something in the simple-bridge binding
<mripard>
but then, your changes would add things to the simple-bridge driver to support properties that are not in the simple-bridge binding
<mripard>
which is kind of weird, and makes simple-bridge more complicated than it could be
<mripard>
even more so since we kind of expect that bridge to have its own driver eventually
<mripard>
and a minimal bridge driver is super simple
<marc|gonzalez>
Dmitry asked that I add my code in simple-bridge, not in a separate driver, unless I misunderstood
<marc|gonzalez>
I DID write a minimal driver
<mripard>
well, Dmitry and I are different persons and we can disagree
<marc|gonzalez>
OK, Dmitry's suggestion made sense as far as code reuse was concerned
<marc|gonzalez>
Maybe I should make the functions extern instead of static?
<marc|gonzalez>
I hear your concern about mixing simple bridge & tdp158 code
<mripard>
extern instead of static? what do you mean?
<marc|gonzalez>
Well, as you can see in patch 4, tdp158 uses 90% of the code in simple bridge for setup
<marc|gonzalez>
So it makes sense to reuse that code
<mripard>
do you have a link to that first version of yours?
<marc|gonzalez>
If not by sharing within the same file, then exporting the function as extern
<marc|gonzalez>
First? meaning not this one?
<mripard>
you had a previous version to support that bridge, right?
<marc|gonzalez>
Right, just a sex
<marc|gonzalez>
sec
<mripard>
(that's typically the things that should go in your cover letter)
<marc|gonzalez>
Proposal was not a format patch submission
<mripard>
yeah, well, I disagree with Dmitry
<mripard>
sometimes, some small amount of code duplication is just better than sharing the code
<mripard>
for 30 LoC, almost entirely boilerplate, it's definitely one of those cases for me
Daanct12 has quit [Quit: WeeChat 4.3.2]
<marc|gonzalez>
Not sure it would be 30 LoC though? It's all of simple bridge, more like 250 duplicated lines
<mripard>
right, because you need to support all its compatibles, EDID readout, hotplug, connector creation, and so on?
Daanct12 has joined #linux-amlogic
Daanct12 has quit [Quit: WeeChat 4.3.2]
<marc|gonzalez>
mripard: I'm not sure how much stuff should be supported, I just assumed simple-bridge supports the minimum stuff required
<mripard>
so you're not sure what you want to support, but you're confident about your driver alleged size?
psydroid2 has joined #linux-amlogic
naoki1 has joined #linux-amlogic
R0nd has joined #linux-amlogic
naoki1 has quit [Client Quit]
naoki1 has joined #linux-amlogic
naoki has quit [*.net *.split]
Rond has quit [*.net *.split]
psydroid has quit [*.net *.split]
Danct12 has quit [*.net *.split]
khilman has quit [*.net *.split]
InFerNo__ is now known as InFerNo_
naoki1 is now known as naoki
R0nd is now known as Rond
psydroid has joined #linux-amlogic
Danct12 has joined #linux-amlogic
khilman has joined #linux-amlogic
<rockosov>
jbrunet: Oh, cool, I will try with regulator as simple amplifier vcc input, thanks a lot
Danct12 has quit [*.net *.split]
khilman has quit [*.net *.split]
Danct12 has joined #linux-amlogic
khilman has joined #linux-amlogic
<marc|gonzalez>
mripard: I'm confident I don't want to send 8 revisions of the same code that just toggles a GPIO.
<marc|gonzalez>
everything else, I have no idea about.
<mripard>
so, maybe, follow the ones that are eventually going to merge it, instead of fighting over it?
<mripard>
because it sure looks like the perfect path to either have 8 revisions, or never getting it merged.
<marc|gonzalez>
Yes. So what is required: drm_bridge_attach / drm_connector_helper_add / drm_connector_init_with_ddc / drm_connector_attach_encoder ?
<marc|gonzalez>
Basically, what parts of simple-bridge should I leave out for a trivial tdp158 driver?
<mripard>
everything you don't need?
<marc|gonzalez>
Is it OK to toggle the GPIO and enable the regulators in the probe function?
<mripard>
Dmitry told you to do so in atomic_enable
<marc|gonzalez>
Yes, but you overruled Dmitry once, so I wanted to know if he was right this time. OK noted.
buzzmarshall has joined #linux-amlogic
<narmstrong>
yeah the right place is in atomic_enable
<marc|gonzalez>
simple-bridge doesn't have atomic_enable, only struct drm_bridge_funcs .enable
<narmstrong>
the old enable is legacy
<narmstrong>
new bridges are expected to only use atomic ops
<marc|gonzalez>
I'm gonna discuss with arnaud if we really should upstream this thing
<marc|gonzalez>
The 3-line hack does what we need
<marc|gonzalez>
I still have all the HDMI stuff stuck in review, and the vdec thing stuck in limbo
<mripard>
"stuck in review" meaning you posted it last thursday?
<mripard>
oh, and you got some reviews on the very same day
<marc|gonzalez>
no, I meant, need to spin a new revision
<marc|gonzalez>
errrr, I don't even know why I'm spamming this channel, the issue is with a qcom part
djrscally has joined #linux-amlogic
<marc|gonzalez>
I guess I'm just tired of rewriting the same uninteresting 40 lines of code
djrscally has quit [Client Quit]
<mripard>
do yourself a favor, stop saying those kinds of things. It just make everyone more frustrated for no particular reason
<mripard>
and it'll just make people less likely to interact with you, so you're the one losing out.
<marc|gonzalez>
You're right. Thanks for taking the time to review.
<marc|gonzalez>
I'm out.
marc|gonzalez has left #linux-amlogic [#linux-amlogic]
<rockosov>
narmstrong: Hello! Neil, do you know the reason why in the uboot upstream A/B slot is stored to misc partition w/o underscore symbol, but board developers add
<rockosov>
add '_' symbol to slot_suffix in the cmdline from the board configuration code?
<rockosov>
It looks like, AOSP wants to see slot name with underscore and uboot provides it in such format, but uboot saves slot name to misc partition without underscore.. What's the trick here? Very strange..
<rockosov>
I see only two types of uboot boards which are using AB slot feature: TI and Meson. That's why I think you know some details about it..
<narmstrong>
rockosov: you should ask mkorpershoek about that, I stopped following a long time ago
<rockosov>
narmstrong: Ah, thank you! sorry for ping
<rockosov>
mkorpershoek: Could you please help to understand such uboot behaviour?
<mkorpershoek>
Looking into it
<narmstrong>
rockosov: no problem, you're welcome to ping me!
<mkorpershoek>
to me this is just how ab_select_slot() is implemented, but I'm reading a bit more
<mkorpershoek>
I agree it's odd, because `struct bootloader_control` has ` char slot_suffix[4];` which should be sufficient to contain "_a\0"
<mkorpershoek>
rockosov: if you look at how misc partition is initialized by U-Boot via ab_control_default() you can see that it sets that to `memcpy(abc->slot_suffix, "a\0\0\0", 4);`
<rockosov>
So, looks like this is a bug, because bootloader_control struct describes this field as 'slot_suffix'
<rockosov>
* The slot_suffix field is used for A/B implementations where the
<rockosov>
* bootloader does not set the androidboot.ro.boot.slot_suffix kernel
<rockosov>
* commandline parameter. This is used by fs_mgr to mount /system and
<rockosov>
* other partitions with the slotselect flag set in fstab. A/B
<rockosov>
* implementations are free to use all 32 bytes and may store private
<rockosov>
* data past the first NUL-byte in this field. It is encouraged, but
<rockosov>
* not mandatory, to use 'struct bootloader_control' described below.
<rockosov>
There is comment from include/android_bootloader_message.h
<mkorpershoek>
rockosov: thanks a bunch for looking into this, always happy to see android support improved in U-boot !
<mkorpershoek>
there are some other A/B related fixes in Google's U-Boot fork I intend to port some time this year
<rockosov>
mkorpershoek: Sure, this year we plan to support more Android flows in the uboot. We plan to transfer our consumer AOSP devices to upstream-based uboot (and kernel actually, so graphic is under question as well)
<rockosov>
mkorpershoek: Should I provide the fixes to your patch series about bootmeth_android? Or you will prepare new version? I can put the comment about it to bootmeth patch email thread
<mkorpershoek>
I will handle it when I come back from vacation, I have to respin bootmeth_android anyways since I need to add some doc
<rockosov>
mkorpershoek: Okay, got it! Thank you for the help!
<mkorpershoek>
You are welcome!
psydroid2 has quit [Remote host closed the connection]