lkml.org 
[lkml]   [2022]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v3 3/4] drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
From
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

\
 
 \ /
  Last update: 2022-06-02 16:50    [W:0.431 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site