Hi,
On Mon, Apr 18, 2022 at 9:58 AM Douglas Anderson <dianders@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>
> Changes in v2:
> - Change is_hpd_asserted() to wait_hpd_asserted()
>
> .../gpu/drm/panel/panel-samsung-atna33xc20.c | 38 ++++++++++++-------
> 1 file changed, 25 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..7f9af3e9aeb8 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;
>
> @@ -90,20 +95,25 @@ static int atana33xc20_resume(struct device *dev)
> return ret;
> 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.
> - */
> 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)
> + 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);
> + else if (p->aux->wait_hpd_asserted)
> + ret = p->aux->wait_hpd_asserted(p->aux, HPD_MAX_US);
> +
> + /*
> + * 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 (!hpd_asserted && (p->hpd_gpio || p->aux->wait_hpd_asserted))
> dev_warn(dev, "Timeout waiting for HPD\n");
Ugh, right after I sent this out I found a bug! :( It should be
checking for "ret", not "hpd_asserted" in the above "if" test. I'll
spin out a quick v3 right away!
-Doug
