Messages in this thread Patch in this message | | | Date | Thu, 2 Jun 2022 17:49:28 +0300 | Subject | Re: [PATCH v3 3/4] drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct drm_dp_aux | From | Dmitry Baryshkov <> |
| |
On 18/04/2022 20:17, Douglas Anderson wrote: > Let's add support for being able to read the HPD pin even if it's > hooked directly to the controller. This will let us take away the > waiting in the AUX transfer functions of the eDP controller drivers. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v3: > - Don't check "hpd_asserted" boolean when unset. > - Handle errors from gpiod_get_value_cansleep() properly. > > Changes in v2: > - Change is_hpd_asserted() to wait_hpd_asserted() > > .../gpu/drm/panel/panel-samsung-atna33xc20.c | 41 +++++++++++++------ > 1 file changed, 28 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > index 20666b6217e7..5ef1b4032c56 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > +++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > @@ -19,6 +19,10 @@ > #include <drm/drm_edid.h> > #include <drm/drm_panel.h> > > +/* T3 VCC to HPD high is max 200 ms */ > +#define HPD_MAX_MS 200 > +#define HPD_MAX_US (HPD_MAX_MS * 1000) > + > struct atana33xc20_panel { > struct drm_panel base; > bool prepared; > @@ -30,6 +34,7 @@ struct atana33xc20_panel { > > struct regulator *supply; > struct gpio_desc *el_on3_gpio; > + struct drm_dp_aux *aux; > > struct edid *edid; > > @@ -79,7 +84,7 @@ static int atana33xc20_suspend(struct device *dev) > static int atana33xc20_resume(struct device *dev) > { > struct atana33xc20_panel *p = dev_get_drvdata(dev); > - bool hpd_asserted = false; > + int hpd_asserted; > int ret; > > /* T12 (Power off time) is min 500 ms */ > @@ -91,20 +96,28 @@ static int atana33xc20_resume(struct device *dev) > p->powered_on_time = ktime_get(); > > /* > - * Handle HPD. Note: if HPD is hooked up to a dedicated pin on the > - * eDP controller then "no_hpd" will be false _and_ "hpd_gpio" will be > - * NULL. It's up to the controller driver to wait for HPD after > - * preparing the panel in that case. > + * Note that it's possible that no_hpd is false, hpd_gpio is > + * NULL, and wait_hpd_asserted is NULL. This is because > + * wait_hpd_asserted() is optional even if HPD is hooked up to > + * a dedicated pin on the eDP controller. In this case we just > + * assume that the controller driver will wait for HPD at the > + * right times. > */ > if (p->no_hpd) { > - /* T3 VCC to HPD high is max 200 ms */ > - msleep(200); > - } else if (p->hpd_gpio) { > - ret = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio, > - hpd_asserted, hpd_asserted, > - 1000, 200000); > - if (!hpd_asserted) > - dev_warn(dev, "Timeout waiting for HPD\n"); > + msleep(HPD_MAX_MS); > + } else { > + if (p->hpd_gpio) { > + ret = readx_poll_timeout(gpiod_get_value_cansleep, > + p->hpd_gpio, hpd_asserted, > + hpd_asserted, 1000, HPD_MAX_US); > + if (hpd_asserted < 0) > + ret = hpd_asserted; > + } else if (p->aux->wait_hpd_asserted) { > + ret = p->aux->wait_hpd_asserted(p->aux, HPD_MAX_US); > + } > + > + if (ret) > + dev_warn(dev, "Error waiting for HPD: %d\n", ret);
I'd suggest reworking this to:
if (p->no_hpd) { msleep(); return 0; }
if (p->hpd_gpio) { ret = readx_poll_timeout(...)
if (ret) dev_warn() return ret; }
if (p->aux->wait_hpd_asserted) { ret = p->aux->wait.... if (ret) dev_warn(...) return ret; }
return 0;
> } > > return 0; > @@ -263,6 +276,8 @@ static int atana33xc20_probe(struct dp_aux_ep_device *aux_ep) > return -ENOMEM; > dev_set_drvdata(dev, panel); > > + panel->aux = aux_ep->aux; > + > panel->supply = devm_regulator_get(dev, "power"); > if (IS_ERR(panel->supply)) > return dev_err_probe(dev, PTR_ERR(panel->supply),
-- With best wishes Dmitry
| |