Hi, On Thu, Apr 29, 2021 at 6:28 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Fri, Apr 30, 2021 at 3:25 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > > > I think pm_runtime_disable(); need to be added there? > > > > I'm a bit confused. You're saying that I need to add > > pm_runtime_disable() to panel_simple_remove()? Doesn't this patch do > > that? > > It does, sorry, too late at night :D > > I was looking at the previous patch and mixed up which was the > patch and the patch to the patch... > > Thanks, apply this! Pushed this one patch. Rest of the series is pending adult supervision. Overall summary: 1. I could probably push some of the early sn65dsi86 cleanup patches in this series since they have Bjorn's review and are pretty much no-ops / simple cleanups, but there's probably not tons gained for shoving those in early. 2. The whole concept of breaking up the patch into sub-drivers has no official Reviewed-by tags yet. Presumably Bjorn will give those a re-review when he has time again. Assuming nobody is really upset about it, I could land those which might unblock some of Bjorn's future PWM work. It would probably be good to get an extra set of eyes on them, though, just so someone else agrees that they're not "too hacky" or anything. IMO it's actually a pretty nice solution, but I'm biased! 3. Laurent and I had a big discussion on #dri-devel yesterday about the EDID reading. He's not totally convinced with the idea of doing this in the panel when the bridge could just do it by itself, but it sounded like he might be coming around. Right now this is waiting on Laurent to have time to get back to this. My summary of the IRC discussion with Laurent (please correct if I got this wrong): a) In general I argued that it was important to be able to provide the EDID and the DDC bus to the panel driver. Providing the EDID to the panel driver allows the panel driver is one of the prerequisites for my proposal for solving the "panel second sourcing" problem [1]. Being able to provide the DDC bus to the panel will likely be important in the eventual solution to Rajeev's problem [2]. b) Today, if we provide the DDC bus to simple-panel then simple-panel will assume it's in charge of reading the EDID. c) Having the panel driver involved in reading the EDID feels like it makes sense to me. The panel driver knows how to power the panel on enough to read the EDID. It also might know extra quirks needed to read the EDID on a given panel. This feels a little cleaner (to me) than just re-using the panel's "prepare" and assuming that a prepared panel was ready for EDID read, though I can see that both may have their advantages. d) Laurent proposed that some eDP controllers might have special ways to read an EDID but might not be able to provide a DDC bus or an i2c bus. If we run into controllers like this then we would be painted into a corner and we'd have to come up with a new solution. This is definitely a good point, though it remains to be seen if this is common with eDP (like Laurent says it is for HDMI). Some eDP panels need custom DDC commands in order to be configured and so hopefully all eDP bridges out there at least provide a DDC bus. It does feel like this could be solved later, though. My patch series is leveraging the existing concept that the panel driver is in charge of reading the EDID if it's given the DDC bus, so it's not creating a new mechanism but instead just continuing to use the existing mechanism. If the existing mechanism doesn't work then it can be improved when there is a need. e) Laurent worried about circular dependencies and wanted to see my solution to the problem before deciding if it was too big of a hack. Hopefully it looks OK since it solves not only this problem but also the HPD GPIO problem and will be important for when Bjorn exports the PWM from the bridge chip. [1] https://lore.kernel.org/lkml/CAD=FV=VZYOMPwQZzWdhJGh5cjJWw_EcM-wQVEivZ-bdGXjPrEQ@xxxxxxxxxxxxxx/ [2] https://lore.kernel.org/r/78c4bd291bd4a17ae2a1d02d0217de43@xxxxxxxxxxxxxx OK, I'll shut up now. ;-) -Doug _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
