| Message ID | 20191215163810.52356-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | drm/i915/dsi: Control panel and backlight enable GPIOs from VBT | expand |
On Sun, Dec 15, 2019 at 5:38 PM Hans de Goede <hdegoede@redhat.com> wrote: > Linus, this series starts with the already discussed pinctrl change to > export the function to unregister a pinctrl-map. We can either merge this > through drm-intel, or you could pick it up and then provide an immutable > branch with it for merging into drm-intel-next. Which option do you prefer? I have created an immutable branch with these changes and pulled it to my "devel" branch for v5.6: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=ib-pinctrl-unreg-mappings Please pull this in and put the other patches on top of that. I had a bit of mess in my subsystems last kernel cycle so I want to avoid that by strictly including all larger commits in my trees. Yours, Linus Walleij
Hi, On 16-12-2019 11:26, Linus Walleij wrote: > On Sun, Dec 15, 2019 at 5:38 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> Linus, this series starts with the already discussed pinctrl change to >> export the function to unregister a pinctrl-map. We can either merge this >> through drm-intel, or you could pick it up and then provide an immutable >> branch with it for merging into drm-intel-next. Which option do you prefer? > > I have created an immutable branch with these changes and pulled it > to my "devel" branch for v5.6: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=ib-pinctrl-unreg-mappings > > Please pull this in and put the other patches on top of that. Great, thank you. Jani, a question about how I am supposed to push this (when this has been also reviewed by some i915 devs and passes CI), can I just do: dim backmerge drm-intel-next-queued linux-pinctrl/ib-pinctrl-unreg-mappings cat other-patches | dim apply-branch drm-intel-next-queued dim push-branch drm-intel-next-queued Or I guess that dim backmerge is reserved for other drm branches only and I should do: dim checkout drm-intel-next-queued git merge linux-pinctrl/ib-pinctrl-unreg-mappings cat other-patches | dim apply-branch drm-intel-next-queued dim push-branch drm-intel-next-queued Or should I leave merging linux-pinctrl/ib-pinctrl-unreg-mappings into drm-intel-next-queued up to you? > I had a bit of mess in my subsystems last kernel cycle so I > want to avoid that by strictly including all larger commits > in my trees. I understand. Regards, Hans
Hi, On 16-12-2019 11:26, Linus Walleij wrote: > On Sun, Dec 15, 2019 at 5:38 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> Linus, this series starts with the already discussed pinctrl change to >> export the function to unregister a pinctrl-map. We can either merge this >> through drm-intel, or you could pick it up and then provide an immutable >> branch with it for merging into drm-intel-next. Which option do you prefer? > > I have created an immutable branch with these changes and pulled it > to my "devel" branch for v5.6: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=ib-pinctrl-unreg-mappings Ugh, taking one last look at the "pinctrl: Export pinctrl_unregister_mappings" patch it is no good, sorry. I just realized that if the mapping has been dupped, the if (maps_node->maps == map) check will never be true, because maps_node->maps is the return value from kmemdup and map is the map originally passed in while registering. Linus, can you please drop this from your -next ? So I see 2 options: 1) Add an orig_map member to maps_node and use that in the comparison, this is IMHO somewhat ugly 2) Add a new pinctrl_register_mappings_no_dup helper and document in pinctrl_unregister_mappings kdoc that it can only be used together with the no_dup variant. I believe that 2 is by far the best option. Linus do you agree or do you have any other suggestions? Regards, Hans
On Mon, Dec 16, 2019 at 12:11 PM Hans de Goede <hdegoede@redhat.com> wrote: > Ugh, taking one last look at the "pinctrl: Export pinctrl_unregister_mappings" > patch it is no good, sorry. Ooops! > Linus, can you please drop this from your -next ? Sure, done. > So I see 2 options: > 1) Add an orig_map member to maps_node and use that in the comparison, > this is IMHO somewhat ugly > > 2) Add a new pinctrl_register_mappings_no_dup helper and document in > pinctrl_unregister_mappings kdoc that it can only be used together > with the no_dup variant. > > I believe that 2 is by far the best option. Linus do you agree or > do you have any other suggestions? What about (3) look for all calls to pinctrl_register_mappings() in the kernel. Hey it is 2 places in total: arch/arm/mach-u300/core.c: pinctrl_register_mappings(u300_pinmux_map, drivers/pinctrl/cirrus/pinctrl-madera-core.c: ret = pinctrl_register_mappings(pdata->gpio_configs, Delete __initdata from the u300 table, the other one seems safe. Fold this into your patch. Go with the original idea. Yours, Linus Walleij
On Sun, Dec 15, 2019 at 05:38:05PM +0100, Hans de Goede wrote: > Hi All, > > This is a new (completely rewritten) version of my patches to make the > i915 code control the SoC panel- and backlight-enable GPIOs on Bay Trail > devices when the VBT indicates that the SoC should be used for backlight > control. This fixes the panel not lighting up on various devices when > booted with a HDMI monitor connected, in which case the firmware skips > initializing the panel as it inits the HDMI instead. > > This series has been tested on; and fixes this issue on; the following models: > > Peaq C1010 > Point of View MOBII TAB-P800W > Point of View MOBII TAB-P1005W > Terra Pad 1061 > Thundersoft TST178 > Yours Y8W81 > > Linus, this series starts with the already discussed pinctrl change to > export the function to unregister a pinctrl-map. We can either merge this > through drm-intel, or you could pick it up and then provide an immutable > branch with it for merging into drm-intel-next. Which option do you prefer? > > Lee, I know you don't like this, but unfortunately this series introcudes > some (other) changes to drivers/mfd/intel_soc_pmic_core.c. The GPIO subsys > allows only one mapping-table per consumer, so in hindsight adding the code > which adds the mapping for the PMIC panel-enable pin to the PMIC mfd driver > was a mistake, as the PMIC code is a provider where as mapping-tables are > per consumer. The 4th patch fixes this by moving the mapping-table to the > i915 code, so that we can also add mappings for some of the pins on the SoC > itself. Since this whole series makes change to the i915 code I plan to > merge this mfd change to the drm-intel tree. FWIW, Lee, I believe there will be no (significant) changes in the driver Hans touched. For the record it seems only Hans is touching drivers for old Intel platforms (such as Baytrail and Cherryview).
Hi, On 16-12-2019 13:16, Linus Walleij wrote: > On Mon, Dec 16, 2019 at 12:11 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> Ugh, taking one last look at the "pinctrl: Export pinctrl_unregister_mappings" >> patch it is no good, sorry. > > Ooops! > >> Linus, can you please drop this from your -next ? > > Sure, done. > >> So I see 2 options: >> 1) Add an orig_map member to maps_node and use that in the comparison, >> this is IMHO somewhat ugly >> >> 2) Add a new pinctrl_register_mappings_no_dup helper and document in >> pinctrl_unregister_mappings kdoc that it can only be used together >> with the no_dup variant. >> >> I believe that 2 is by far the best option. Linus do you agree or >> do you have any other suggestions? > > What about (3) look for all calls to pinctrl_register_mappings() > in the kernel. > > Hey it is 2 places in total: > arch/arm/mach-u300/core.c: pinctrl_register_mappings(u300_pinmux_map, > drivers/pinctrl/cirrus/pinctrl-madera-core.c: ret = > pinctrl_register_mappings(pdata->gpio_configs, > > Delete __initdata from the u300 table, the other one seems > safe. Fold this into your patch. > > Go with the original idea. That indeed sounds like a cleaner solution I will prepare a new version of the patch (and this series for the i915 CI) with this approach. Thanks & Regards, Hans
On Mon, 16 Dec 2019, Andy Shevchenko wrote: > On Sun, Dec 15, 2019 at 05:38:05PM +0100, Hans de Goede wrote: > > Hi All, > > > > This is a new (completely rewritten) version of my patches to make the > > i915 code control the SoC panel- and backlight-enable GPIOs on Bay Trail > > devices when the VBT indicates that the SoC should be used for backlight > > control. This fixes the panel not lighting up on various devices when > > booted with a HDMI monitor connected, in which case the firmware skips > > initializing the panel as it inits the HDMI instead. > > > > This series has been tested on; and fixes this issue on; the following models: > > > > Peaq C1010 > > Point of View MOBII TAB-P800W > > Point of View MOBII TAB-P1005W > > Terra Pad 1061 > > Thundersoft TST178 > > Yours Y8W81 > > > > Linus, this series starts with the already discussed pinctrl change to > > export the function to unregister a pinctrl-map. We can either merge this > > through drm-intel, or you could pick it up and then provide an immutable > > branch with it for merging into drm-intel-next. Which option do you prefer? > > > > Lee, I know you don't like this, but unfortunately this series introcudes > > some (other) changes to drivers/mfd/intel_soc_pmic_core.c. The GPIO subsys > > allows only one mapping-table per consumer, so in hindsight adding the code > > which adds the mapping for the PMIC panel-enable pin to the PMIC mfd driver > > was a mistake, as the PMIC code is a provider where as mapping-tables are > > per consumer. The 4th patch fixes this by moving the mapping-table to the > > i915 code, so that we can also add mappings for some of the pins on the SoC > > itself. Since this whole series makes change to the i915 code I plan to > > merge this mfd change to the drm-intel tree. > > FWIW, Lee, I believe there will be no (significant) changes in the driver Hans > touched. For the record it seems only Hans is touching drivers for old Intel > platforms (such as Baytrail and Cherryview). More exceptions, yay! Again, in *this* case, it's probably fine. What I want to know is; what happens when it's not fine? What happens when you or someone else starts changing MFD and DRM on more active files? Then I will have to insist on an immutable branch. So it would be better for the DRM tree to be able to handle that use-case sooner rather than later, regardless of who has the most churn.