Re: [PATCH 1/3] pinctrl: Intel: add RX invertion config

Re: [PATCH 1/3] pinctrl: Intel: add RX invertion config

[Daniel Vetter]

On Wed, Mar 16, 2016 at 01:27:49PM +0100, Linus Walleij wrote:
> 
> On Tue, Mar 15, 2016 at 3:17 AM, Zheng, Qi <qi.zheng@intel.com> wrote:
> >> On Mon, Mar 14, 2016 at 9:56 AM, Zheng, Qi <qi.zheng@intel.com> wrote:
> >
> >> The "pi330532" device on Broxton requires this function to manually
> >> trigger an GPIO input interrupt.
> > (...)
> >> We have gone through this requirement from CHT to BXT, there was no
> >> other better way to simulate the GPIO interrupt for the use of those devices.
> >
> >> I what you want is to trigger IRQs on GPIO lines using software we
> >> need to add that to the GPIOlib subsystem, so this register gets accessed
> >> from the GPIO side of things, not through pin control I think?
> >
> > We have pinctrl control map locally.
> > The RX inversion is implemented by pinctrl control calls, pinctrl_pm_select_default_state
> > and pinctrl_pm_select_sleep_state.
> >
> >> We have so many diverse function pointers in the gpiochip, so ability to
> >> trigger/test IRQs from software is certainly not a burden.
> >
> >> I don't understand the real-world usecase though, please explain what kind of problem
> >> this is trying to overcome? Why does this pi330532 driver need to do that, why can
> >> it not just inform the driver that needs this interrupt that it should wake up, e.g by
> >> using a notification or just an open-coded function call or whatever?
> >
> > According to the pi330532 driver owner,
> > "
> > we needed this support to simulate the HPD interrupt behavior as we don’t have
> > dedicated interrupt line for Type-C DP HPD.
> 
> - What is a HPD interrupt?

hotplug interrupt, fires when you plug in a cable.

> - What is a Type-C DP HPD?

usb type C connector can multiplex both DisplayPort and USB, you need to
renegotiate the lane splitting every time the sink changes, i.e. on each
hotplug.

> - Again why can't you just use a notifier or function call?

Because windows sucks, hence all that coordination happens through hw
forwarded interrupts and magic registers. Same horror story on the sound
side, where the sound driver needs to know what kind of PCM stream the
monitor can take. It's hilarious. Except when they screw up the design and
then need to fake parts of it in software.

In sound we've switched over to a proper sw interface, and we tie the
different parts (drm graphics driver and alsa snd driver) using the
component.c framework.

> > We don’t have any notifications mechasism b/w USB and display/Gfx stack and
> > also not the ideal way to handle.  HPD toggling is the preferred approach suggested
> > by VPG and HW teams to meet timing requirements also.
> 
> What is VPG? Now it seems Intel's internal organization is being used as
> part of the argument to get this change in and that makes me a bit
> annoyed.
> 
> If there is no good notification mechanism then implement one instead
> of starting to software-generate hardware interrupts.
> 
> I also start to get the feeling that these USB and display stacks
> you are referring to are not the upstream versions.

There was chat of usb type C support for forever, but I was always
promised that we don't need any interactions on the sw side and it's all
magic in hw. There hasn't been any real design discussions in the open
source group. VPG is the hw/windows group and generally comes up with
"interesting" designs not suitable for upstream.

Feel free to just nack this stuff, and please cc intel-gfx/dri-devel in
the future (since I tend to ignore everything that's not cc'ed to mailing
lists I don't care about, even when I'm on cc personally). I've added them
all to cc.

Cheers, Daniel


> > static void hpd_trigger(struct pi3usb30532_mux *chip, int state)
> > {
> >         dev_info(&chip->client->dev, "[HPD trigger] state : %d\n", state);
> >
> >         if (state)
> >                 pinctrl_pm_select_default_state(chip->dev);
> >         else
> >                 pinctrl_pm_select_sleep_state(chip->dev);
> > }
> 
> Can we get the *TECHNICAL* explanation of why this thing needs
> to be done instead of using a notifier or function call?
> 
> Yours,
> Linus Walleij

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] pinctrl: Intel: add RX invertion config

[Linus Walleij]

On Wed, Mar 16, 2016 at 2:34 PM, Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> wrote:
> On Wed, Mar 16, 2016 at 01:27:49PM +0100, Linus Walleij wrote:

>> - What is a HPD interrupt?
>
> hotplug interrupt, fires when you plug in a cable.
>
>> - What is a Type-C DP HPD?
>
> usb type C connector can multiplex both DisplayPort and USB, you need to
> renegotiate the lane splitting every time the sink changes, i.e. on each
> hotplug.

OK I understand, thanks a lot!

>> - Again why can't you just use a notifier or function call?
>
> Because windows sucks, hence all that coordination happens through hw
> forwarded interrupts and magic registers. Same horror story on the sound
> side, where the sound driver needs to know what kind of PCM stream the
> monitor can take. It's hilarious. Except when they screw up the design and
> then need to fake parts of it in software.

So the story is something like that these IRQs have been put into
hardware in order to compensate for flaws in Windows device driver
model, I see.

If there are such special registers in some hardware I guess I'm all for
implementing some generic quirk in gpiolib for people who need to
software-trigger GPIO IRQs. Could be good for testing too, as there
are such registers in ARMs PL061 GPIO controller for test, just so as
to simulate a GPIO IRQ.

gpiod_trig_irq() would work with me, I'm happy to support whatever
the GPIO hardware can do usually.

> In sound we've switched over to a proper sw interface, and we tie the
> different parts (drm graphics driver and alsa snd driver) using the
> component.c framework.

Hm is that solution or something similar proper for USB connector
as well I wonder... I was thinking about just adding $random_notifier
but maybe that is a bit ugly :/

>> What is VPG? Now it seems Intel's internal organization is being used as
>> part of the argument to get this change in and that makes me a bit
>> annoyed.
(...)
> There was chat of usb type C support for forever, but I was always
> promised that we don't need any interactions on the sw side and it's all
> magic in hw. There hasn't been any real design discussions in the open
> source group. VPG is the hw/windows group and generally comes up with
> "interesting" designs not suitable for upstream.
>
> Feel free to just nack this stuff, and please cc intel-gfx/dri-devel in
> the future (since I tend to ignore everything that's not cc'ed to mailing
> lists I don't care about, even when I'm on cc personally). I've added them
> all to cc.

Thanks a lot Daniel, I understand better now. I'm not really against
adding this "interesting" workaround if that is how Windows works,
we usually have to go by their standards. From the GPIO point
of view it is OK, just something the GPIO can do. I would be more
worried about what the USB PHY maintainer (Felipe) is going to say
about this.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Intel-gfx] [PATCH 1/3] pinctrl: Intel: add RX invertion config

[Jani Nikula]

On Thu, 17 Mar 2016, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Mar 16, 2016 at 2:34 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Mar 16, 2016 at 01:27:49PM +0100, Linus Walleij wrote:
>
>>> - What is a HPD interrupt?
>>
>> hotplug interrupt, fires when you plug in a cable.
>>
>>> - What is a Type-C DP HPD?
>>
>> usb type C connector can multiplex both DisplayPort and USB, you need to
>> renegotiate the lane splitting every time the sink changes, i.e. on each
>> hotplug.
>
> OK I understand, thanks a lot!
>
>>> - Again why can't you just use a notifier or function call?
>>
>> Because windows sucks, hence all that coordination happens through hw
>> forwarded interrupts and magic registers. Same horror story on the sound
>> side, where the sound driver needs to know what kind of PCM stream the
>> monitor can take. It's hilarious. Except when they screw up the design and
>> then need to fake parts of it in software.
>
> So the story is something like that these IRQs have been put into
> hardware in order to compensate for flaws in Windows device driver
> model, I see.
>
> If there are such special registers in some hardware I guess I'm all for
> implementing some generic quirk in gpiolib for people who need to
> software-trigger GPIO IRQs. Could be good for testing too, as there
> are such registers in ARMs PL061 GPIO controller for test, just so as
> to simulate a GPIO IRQ.
>
> gpiod_trig_irq() would work with me, I'm happy to support whatever
> the GPIO hardware can do usually.
>
>> In sound we've switched over to a proper sw interface, and we tie the
>> different parts (drm graphics driver and alsa snd driver) using the
>> component.c framework.
>
> Hm is that solution or something similar proper for USB connector
> as well I wonder... I was thinking about just adding $random_notifier
> but maybe that is a bit ugly :/
>
>>> What is VPG? Now it seems Intel's internal organization is being used as
>>> part of the argument to get this change in and that makes me a bit
>>> annoyed.
> (...)
>> There was chat of usb type C support for forever, but I was always
>> promised that we don't need any interactions on the sw side and it's all
>> magic in hw. There hasn't been any real design discussions in the open
>> source group. VPG is the hw/windows group and generally comes up with
>> "interesting" designs not suitable for upstream.
>>
>> Feel free to just nack this stuff, and please cc intel-gfx/dri-devel in
>> the future (since I tend to ignore everything that's not cc'ed to mailing
>> lists I don't care about, even when I'm on cc personally). I've added them
>> all to cc.
>
> Thanks a lot Daniel, I understand better now. I'm not really against
> adding this "interesting" workaround if that is how Windows works,
> we usually have to go by their standards. From the GPIO point
> of view it is OK, just something the GPIO can do. I would be more
> worried about what the USB PHY maintainer (Felipe) is going to say
> about this.

Adding Felipe's current address. Considering the new domain part of the
address, I'm hopeful we can sort this out. ;)

BR,
Jani.

>
> Yours,
> Linus Walleij
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center