The double reset power-on sequence is a workaround for the hardware
flaw in some chip that SPI Clock output glitch and cause internal MPU
unable to read firmware correctly. The sequence is suggested in ps8640
application note.
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 49107a6cdac18..d7483c13c569b 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
gpiod_set_value(ps_bridge->gpio_reset, 1);
usleep_range(2000, 2500);
gpiod_set_value(ps_bridge->gpio_reset, 0);
+ /* Double reset for T4 and T5 */
+ msleep(50);
+ gpiod_set_value(ps_bridge->gpio_reset, 1);
+ msleep(50);
+ gpiod_set_value(ps_bridge->gpio_reset, 0);
/*
* Mystery 200 ms delay for the "MCU to be ready". It's unclear if
--
2.37.1.595.g718a3a8f04-goog
Hsin-Yi Wang <[email protected]> 於 2022年8月15日 週一 下午5:39寫道：
>
> The double reset power-on sequence is a workaround for the hardware
> flaw in some chip that SPI Clock output glitch and cause internal MPU
> unable to read firmware correctly. The sequence is suggested in ps8640
> application note.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
Reviewed-by: Rock Chiu <[email protected]>
>
> ---
>
> drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 49107a6cdac18..d7483c13c569b 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
> gpiod_set_value(ps_bridge->gpio_reset, 1);
> usleep_range(2000, 2500);
> gpiod_set_value(ps_bridge->gpio_reset, 0);
> + /* Double reset for T4 and T5 */
> + msleep(50);
> + gpiod_set_value(ps_bridge->gpio_reset, 1);
> + msleep(50);
> + gpiod_set_value(ps_bridge->gpio_reset, 0);
>
> /*
> * Mystery 200 ms delay for the "MCU to be ready". It's unclear if
> --
> 2.37.1.595.g718a3a8f04-goog
>
Hi,
On Mon, Aug 15, 2022 at 2:39 AM Hsin-Yi Wang <[email protected]> wrote:
>
> The double reset power-on sequence is a workaround for the hardware
> flaw in some chip that SPI Clock output glitch and cause internal MPU
> unable to read firmware correctly. The sequence is suggested in ps8640
> application note.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
> ---
> drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 49107a6cdac18..d7483c13c569b 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
> gpiod_set_value(ps_bridge->gpio_reset, 1);
> usleep_range(2000, 2500);
> gpiod_set_value(ps_bridge->gpio_reset, 0);
> + /* Double reset for T4 and T5 */
> + msleep(50);
> + gpiod_set_value(ps_bridge->gpio_reset, 1);
> + msleep(50);
> + gpiod_set_value(ps_bridge->gpio_reset, 0);
We really need another 100 ms here? ps8640 is already quite slow at
powering itself up and that has a real user impact. Why was it only
2.5 ms for the first reset and 50 ms for the second?
-Doug
On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
<[email protected]> wrote:
>
> How does T4/T5 impact the real case? We talked previously the T4/T5
> shouldn't cause user impact.
> Do we have testing data from ODM?
>
Please leave comments below the previous comment's headline.
I'm confused. The reason I upstreamed this patch is because this is in
your application note and you asked us to help upstream it. Now do you
mean that we don't need T4 and T5?
> Rock Chiu
>
> Hsin-Yi Wang <[email protected]> 於 2022年8月18日 週四 上午10:43寫道：
> >
> > On Thu, Aug 18, 2022 at 6:54 AM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Aug 15, 2022 at 2:39 AM Hsin-Yi Wang <[email protected]> wrote:
> > > >
> > > > The double reset power-on sequence is a workaround for the hardware
> > > > flaw in some chip that SPI Clock output glitch and cause internal MPU
> > > > unable to read firmware correctly. The sequence is suggested in ps8640
> > > > application note.
> > > >
> > > > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> > > > index 49107a6cdac18..d7483c13c569b 100644
> > > > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> > > > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> > > > @@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
> > > > gpiod_set_value(ps_bridge->gpio_reset, 1);
> > > > usleep_range(2000, 2500);
> > > > gpiod_set_value(ps_bridge->gpio_reset, 0);
> > > > + /* Double reset for T4 and T5 */
> > > > + msleep(50);
> > > > + gpiod_set_value(ps_bridge->gpio_reset, 1);
> > > > + msleep(50);
> > > > + gpiod_set_value(ps_bridge->gpio_reset, 0);
> > >
> > > We really need another 100 ms here? ps8640 is already quite slow at
> > > powering itself up and that has a real user impact. Why was it only
> > > 2.5 ms for the first reset and 50 ms for the second?
> > >
> >
> > The T4 and T5 are required by Parade. I'm wondering if they can
> > shorten the 200ms below:
> >
> > /*
> > * Mystery 200 ms delay for the "MCU to be ready". It's unclear if
> > * this is truly necessary since the MCU will already signal that
> > * things are "good to go" by signaling HPD on "gpio 9". See
> > * _ps8640_wait_hpd_asserted(). For now we'll keep this mystery delay
> > * just in case.
> > */
> > msleep(200);
> >
> > Does this have to wait 200ms? Can it shorten to 100 due to the
> > additional 100ms from T4 and T5?
> >
> > > -Doug
How does T4/T5 impact the real case? We talked previously the T4/T5
shouldn't cause user impact.
Do we have testing data from ODM?
Rock Chiu
Hsin-Yi Wang <[email protected]> 於 2022年8月18日 週四 上午10:43寫道：
>
> On Thu, Aug 18, 2022 at 6:54 AM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, Aug 15, 2022 at 2:39 AM Hsin-Yi Wang <[email protected]> wrote:
> > >
> > > The double reset power-on sequence is a workaround for the hardware
> > > flaw in some chip that SPI Clock output glitch and cause internal MPU
> > > unable to read firmware correctly. The sequence is suggested in ps8640
> > > application note.
> > >
> > > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > > ---
> > > drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> > > index 49107a6cdac18..d7483c13c569b 100644
> > > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> > > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> > > @@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
> > > gpiod_set_value(ps_bridge->gpio_reset, 1);
> > > usleep_range(2000, 2500);
> > > gpiod_set_value(ps_bridge->gpio_reset, 0);
> > > + /* Double reset for T4 and T5 */
> > > + msleep(50);
> > > + gpiod_set_value(ps_bridge->gpio_reset, 1);
> > > + msleep(50);
> > > + gpiod_set_value(ps_bridge->gpio_reset, 0);
> >
> > We really need another 100 ms here? ps8640 is already quite slow at
> > powering itself up and that has a real user impact. Why was it only
> > 2.5 ms for the first reset and 50 ms for the second?
> >
>
> The T4 and T5 are required by Parade. I'm wondering if they can
> shorten the 200ms below:
>
> /*
> * Mystery 200 ms delay for the "MCU to be ready". It's unclear if
> * this is truly necessary since the MCU will already signal that
> * things are "good to go" by signaling HPD on "gpio 9". See
> * _ps8640_wait_hpd_asserted(). For now we'll keep this mystery delay
> * just in case.
> */
> msleep(200);
>
> Does this have to wait 200ms? Can it shorten to 100 due to the
> additional 100ms from T4 and T5?
>
> > -Doug
On Thu, Aug 18, 2022 at 6:54 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Mon, Aug 15, 2022 at 2:39 AM Hsin-Yi Wang <[email protected]> wrote:
> >
> > The double reset power-on sequence is a workaround for the hardware
> > flaw in some chip that SPI Clock output glitch and cause internal MPU
> > unable to read firmware correctly. The sequence is suggested in ps8640
> > application note.
> >
> > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/parade-ps8640.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> > index 49107a6cdac18..d7483c13c569b 100644
> > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> > @@ -375,6 +375,11 @@ static int __maybe_unused ps8640_resume(struct device *dev)
> > gpiod_set_value(ps_bridge->gpio_reset, 1);
> > usleep_range(2000, 2500);
> > gpiod_set_value(ps_bridge->gpio_reset, 0);
> > + /* Double reset for T4 and T5 */
> > + msleep(50);
> > + gpiod_set_value(ps_bridge->gpio_reset, 1);
> > + msleep(50);
> > + gpiod_set_value(ps_bridge->gpio_reset, 0);
>
> We really need another 100 ms here? ps8640 is already quite slow at
> powering itself up and that has a real user impact. Why was it only
> 2.5 ms for the first reset and 50 ms for the second?
>
The T4 and T5 are required by Parade. I'm wondering if they can
shorten the 200ms below:
/*
* Mystery 200 ms delay for the "MCU to be ready". It's unclear if
* this is truly necessary since the MCU will already signal that
* things are "good to go" by signaling HPD on "gpio 9". See
* _ps8640_wait_hpd_asserted(). For now we'll keep this mystery delay
* just in case.
*/
msleep(200);
Does this have to wait 200ms? Can it shorten to 100 due to the
additional 100ms from T4 and T5?
> -Doug
Hi,
On Wed, Aug 17, 2022 at 8:22 PM Hsin-Yi Wang <[email protected]> wrote:
>
> On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
> <[email protected]> wrote:
> >
> > How does T4/T5 impact the real case? We talked previously the T4/T5
> > shouldn't cause user impact.
> > Do we have testing data from ODM?
> >
> Please leave comments below the previous comment's headline.
>
> I'm confused. The reason I upstreamed this patch is because this is in
> your application note and you asked us to help upstream it. Now do you
> mean that we don't need T4 and T5?
I think Rock is asking what problems the extra delay is causing. In
other words: why do we care about keeping these delays short?
The answer is that it affects boot speed and also resume speed of
devices. Adding these two delays here means that there's an extra 100
ms before the user can see something on the screen. That may not seem
like a lot, but those kinds of delays add up quickly. At least on
Chromebooks, booting quickly is always a big goal.
-Doug
Hi,
On Thu, Aug 18, 2022 at 8:03 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, Aug 17, 2022 at 8:22 PM Hsin-Yi Wang <[email protected]> wrote:
> >
> > On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
> > <[email protected]> wrote:
> > >
> > > How does T4/T5 impact the real case? We talked previously the T4/T5
> > > shouldn't cause user impact.
> > > Do we have testing data from ODM?
> > >
> > Please leave comments below the previous comment's headline.
> >
> > I'm confused. The reason I upstreamed this patch is because this is in
> > your application note and you asked us to help upstream it. Now do you
> > mean that we don't need T4 and T5?
>
> I think Rock is asking what problems the extra delay is causing. In
> other words: why do we care about keeping these delays short?
>
> The answer is that it affects boot speed and also resume speed of
> devices. Adding these two delays here means that there's an extra 100
> ms before the user can see something on the screen. That may not seem
> like a lot, but those kinds of delays add up quickly. At least on
> Chromebooks, booting quickly is always a big goal.
While I'm not very happy with this change and I don't really
understand why these delays need to be so long, if folks are really
certain that we need them and can't make them shorter then I guess we
can land it. I'll wait a few more days in case anyone wants to chime
in with their thoughts.
-Doug
Hi,
On Mon, Aug 22, 2022 at 9:33 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Thu, Aug 18, 2022 at 8:03 AM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Wed, Aug 17, 2022 at 8:22 PM Hsin-Yi Wang <[email protected]> wrote:
> > >
> > > On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
> > > <[email protected]> wrote:
> > > >
> > > > How does T4/T5 impact the real case? We talked previously the T4/T5
> > > > shouldn't cause user impact.
> > > > Do we have testing data from ODM?
> > > >
> > > Please leave comments below the previous comment's headline.
> > >
> > > I'm confused. The reason I upstreamed this patch is because this is in
> > > your application note and you asked us to help upstream it. Now do you
> > > mean that we don't need T4 and T5?
> >
> > I think Rock is asking what problems the extra delay is causing. In
> > other words: why do we care about keeping these delays short?
> >
> > The answer is that it affects boot speed and also resume speed of
> > devices. Adding these two delays here means that there's an extra 100
> > ms before the user can see something on the screen. That may not seem
> > like a lot, but those kinds of delays add up quickly. At least on
> > Chromebooks, booting quickly is always a big goal.
>
> While I'm not very happy with this change and I don't really
> understand why these delays need to be so long, if folks are really
> certain that we need them and can't make them shorter then I guess we
> can land it. I'll wait a few more days in case anyone wants to chime
> in with their thoughts.
I'll continue to grumble, but I did push it.
55453c0914d9 drm/bridge: ps8640: Add double reset T4 and T5 to power-on sequence
I pushed to "drm-misc-next" and not "drm-misc-fixes". It doesn't feel
massively urgent since apparently we've been without the
"double-reset" for years and having the extra bake time feels like the
better way to lean.
-Doug
+ Jason.
-Rock
Doug Anderson <[email protected]> 於 2022年8月30日 週二 清晨5:49寫道：
>
> Hi,
>
> On Mon, Aug 22, 2022 at 9:33 AM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Thu, Aug 18, 2022 at 8:03 AM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Aug 17, 2022 at 8:22 PM Hsin-Yi Wang <[email protected]> wrote:
> > > >
> > > > On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
> > > > <[email protected]> wrote:
> > > > >
> > > > > How does T4/T5 impact the real case? We talked previously the T4/T5
> > > > > shouldn't cause user impact.
> > > > > Do we have testing data from ODM?
> > > > >
> > > > Please leave comments below the previous comment's headline.
> > > >
> > > > I'm confused. The reason I upstreamed this patch is because this is in
> > > > your application note and you asked us to help upstream it. Now do you
> > > > mean that we don't need T4 and T5?
> > >
> > > I think Rock is asking what problems the extra delay is causing. In
> > > other words: why do we care about keeping these delays short?
> > >
> > > The answer is that it affects boot speed and also resume speed of
> > > devices. Adding these two delays here means that there's an extra 100
> > > ms before the user can see something on the screen. That may not seem
> > > like a lot, but those kinds of delays add up quickly. At least on
> > > Chromebooks, booting quickly is always a big goal.
> >
> > While I'm not very happy with this change and I don't really
> > understand why these delays need to be so long, if folks are really
> > certain that we need them and can't make them shorter then I guess we
> > can land it. I'll wait a few more days in case anyone wants to chime
> > in with their thoughts.
>
> I'll continue to grumble, but I did push it.
>
> 55453c0914d9 drm/bridge: ps8640: Add double reset T4 and T5 to power-on sequence
>
> I pushed to "drm-misc-next" and not "drm-misc-fixes". It doesn't feel
> massively urgent since apparently we've been without the
> "double-reset" for years and having the extra bake time feels like the
> better way to lean.
>
> -Doug
The chip is functional ready 200ms after the chip reset. The 200ms is
mainly for loading firmware from spi rom.
-Jason
On Thu, Sep 8, 2022 at 8:32 AM Rock Chiu
<[email protected]> wrote:
>
> + Jason.
>
> -Rock
>
> Doug Anderson <[email protected]> 於 2022年8月30日 週二 清晨5:49寫道：
> >
> > Hi,
> >
> > On Mon, Aug 22, 2022 at 9:33 AM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Aug 18, 2022 at 8:03 AM Doug Anderson <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Wed, Aug 17, 2022 at 8:22 PM Hsin-Yi Wang <[email protected]> wrote:
> > > > >
> > > > > On Thu, Aug 18, 2022 at 11:19 AM Rock Chiu
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > How does T4/T5 impact the real case? We talked previously the T4/T5
> > > > > > shouldn't cause user impact.
> > > > > > Do we have testing data from ODM?
> > > > > >
> > > > > Please leave comments below the previous comment's headline.
> > > > >
> > > > > I'm confused. The reason I upstreamed this patch is because this is in
> > > > > your application note and you asked us to help upstream it. Now do you
> > > > > mean that we don't need T4 and T5?
> > > >
> > > > I think Rock is asking what problems the extra delay is causing. In
> > > > other words: why do we care about keeping these delays short?
> > > >
> > > > The answer is that it affects boot speed and also resume speed of
> > > > devices. Adding these two delays here means that there's an extra 100
> > > > ms before the user can see something on the screen. That may not seem
> > > > like a lot, but those kinds of delays add up quickly. At least on
> > > > Chromebooks, booting quickly is always a big goal.
> > >
> > > While I'm not very happy with this change and I don't really
> > > understand why these delays need to be so long, if folks are really
> > > certain that we need them and can't make them shorter then I guess we
> > > can land it. I'll wait a few more days in case anyone wants to chime
> > > in with their thoughts.
> >
> > I'll continue to grumble, but I did push it.
> >
> > 55453c0914d9 drm/bridge: ps8640: Add double reset T4 and T5 to power-on sequence
> >
> > I pushed to "drm-misc-next" and not "drm-misc-fixes". It doesn't feel
> > massively urgent since apparently we've been without the
> > "double-reset" for years and having the extra bake time feels like the
> > better way to lean.
> >
> > -Doug