From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-20.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1E7C3C2BBCF for ; Fri, 18 Dec 2020 14:47:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DFC2423B27 for ; Fri, 18 Dec 2020 14:47:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728053AbgLROqy (ORCPT ); Fri, 18 Dec 2020 09:46:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34432 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727981AbgLROqx (ORCPT ); Fri, 18 Dec 2020 09:46:53 -0500 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A5EEDC061285 for ; Fri, 18 Dec 2020 06:46:12 -0800 (PST) Received: by mail-wr1-x430.google.com with SMTP id 91so2438348wrj.7 for ; Fri, 18 Dec 2020 06:46:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IOS7/z9Qy+orQvgN4m7ZcdCgVVfUTOFkUvXr8iUDtNQ=; b=jP4+qMylQ3mhIfQvzLOdmkmIohRItctCA1AYJFdDpD6neUt/k85ZMDI4i2OzBVHyT8 9nZ8g3l6IpArhkwp9iYEmf9uY02iLC5C8AVgXOkEOt7FeJlJlwv9EeOc7LDDxCYqxLsR 6nbfPcHjsDCI2lX6zoUbekAqhOBvcrCIGVw+gKlfwdYYe4LxGRM1OEJ5G6BWFiQMl/zr kTRisF/pNz/FW6A2LcC3KDrDDUM8wl+g8CFiZAjD0xC/vkgdkVrJq+Hxz1ZW/sTWIl9v lHLPbwplwfadoeKNL3pYmKNleHr7uoNPsZ5I2f+iiEwzOM8u96WE5ZpnZhcKRgMS3wKM QWKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IOS7/z9Qy+orQvgN4m7ZcdCgVVfUTOFkUvXr8iUDtNQ=; b=WWAazbbI4bBEMLbgzrBWczeKuPABWdK2hGtxwsBx1iD7f9/ixNUJP08hZeXza5ZcXb uJe2TY033S7EMyT8Sz2Up8wyD4kPIKdW4rq0tUOzGiW6DNIKXnGomIYcTbDHaB6yGFwY ZsS//ZQgdTdaASwlaaXXiwEMFT9M1yG+eBn7tOxOfrzQ4LaHXYbWcbidm00V+fTb9K1b Yq8PU+L4CTFsKWhp/sE64N66/+ruUnI/QbUwwQtmyf1NVX+QURbrfmI/JZMR7CX9Iy4V dhZ3Zn6+tf9sLpl+55scgCgC6NPTCicr/P44AW98YqXrAwMA1i0l+8rlPBXaZFT8vNWN C5bA== X-Gm-Message-State: AOAM5324Cdpgy8oTv28NI/NwJsy3Iy5VbERHjZ7PZ1ikArrQkTVcmB+g LN81MjfqNMGBobDz3mium/EvC6d/08nOyjdUrBPkGA== X-Google-Smtp-Source: ABdhPJwtPel488pKG+titVqvbHsJDbSA8DLgQ+5Gid39Aj6tn9e5V4SXOrJcLJdMUB9QQY12ijJ55Y7nrivzD5nE7ec= X-Received: by 2002:adf:f7d2:: with SMTP id a18mr4827679wrq.47.1608302771328; Fri, 18 Dec 2020 06:46:11 -0800 (PST) MIME-Version: 1.0 References: <20201210134648.272857-1-maxime@cerno.tech> <20201210134648.272857-6-maxime@cerno.tech> In-Reply-To: From: Dave Stevenson Date: Fri, 18 Dec 2020 14:45:54 +0000 Message-ID: Subject: Re: [PATCH 05/15] drm/vc4: hdmi: Restore cec physical address on reconnect To: Maxime Ripard Cc: Eric Anholt , Maarten Lankhorst , Thomas Zimmermann , Daniel Vetter , David Airlie , Jason Cooper , bcm-kernel-feedback-list@broadcom.com, linux-arm-kernel@lists.infradead.org, Marc Zyngier , Linux Media Mailing List , Hans Verkuil , LKML , Mauro Carvalho Chehab , Thomas Gleixner , linux-rpi-kernel@lists.infradead.org, DRI Development , Dom Cobley Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 18 Dec 2020 at 14:21, Dave Stevenson wrote: > > Hi Maxime & Dom > > On Thu, 10 Dec 2020 at 13:47, Maxime Ripard wrote: > > > > From: Dom Cobley > > > > Currently we call cec_phys_addr_invalidate on a hotplug deassert. > > That may be due to a TV power cycling, or an AVR being switched > > on (and switching edid). > > > > This makes CEC unusable since our controller wouldn't have a physical > > address anymore. > > > > Set it back up again on the hotplug assert. > > > > Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support") > > Signed-off-by: Dom Cobley > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++-------- > > 1 file changed, 17 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > index 28b78ea885ea..eff3bac562c6 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -136,20 +136,29 @@ static enum drm_connector_status > > vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) > > { > > struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); > > + bool connected = false; > > > > if (vc4_hdmi->hpd_gpio) { > > if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^ > > vc4_hdmi->hpd_active_low) > > - return connector_status_connected; > > - cec_phys_addr_invalidate(vc4_hdmi->cec_adap); > > - return connector_status_disconnected; > > - } > > - > > - if (drm_probe_ddc(vc4_hdmi->ddc)) > > - return connector_status_connected; > > - > > + connected = true; > > + } else if (drm_probe_ddc(vc4_hdmi->ddc)) > > + connected = true; > > if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) > > This needs to become an "else if(...". > It used to be that all the other paths would return, so were mutually > exclusive to this. Now they set a thing and keep going we need to > avoid reading the register should there be a HPD gpio or the ddc probe > succeeds. > Memory says that otherwise Pi3 always reports connected. > > I fixed this in a downstream patch already - > https://github.com/raspberrypi/linux/commit/d345caec1e9b2317b9cd7eb5b92ae453a0d3e98c > > Otherwise fine. > > Dave > > > + connected = true; > > + if (connected) { > > + if (connector->status != connector_status_connected) { > > + struct edid *edid = drm_get_edid(connector, vc4_hdmi->ddc); > > + > > + if (edid) { > > + cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); > > + vc4_hdmi->encoder.hdmi_monitor = drm_detect_hdmi_monitor(edid); > > + drm_connector_update_edid_property(connector, edid); Actually looking at this again in the context of the other changes, do we need to call drm_connector_update_edid_property() here? We've just called drm_get_edid() to get the edid, and that calls drm_connector_update_edid_property() as well [1] Updating vc4_hdmi->encoder.hdmi_monitor may be necessary. It's otherwise done in vc4_hdmi_connector_get_modes, which I sort of expect to be called almost immediately by the framework when connector_detect returns "connected". I haven't checked if that is guaranteed though. vc4_hdmi_connector_get_modes also includes a manual call to drm_connector_update_edid_property after having just called drm_get_edid, so that one feels redundant too. Dave [1] https://elixir.bootlin.com/linux/v5.10/source/drivers/gpu/drm/drm_edid.c#L2059 > > + kfree(edid); > > + } > > + } > > return connector_status_connected; > > + } > > cec_phys_addr_invalidate(vc4_hdmi->cec_adap); > > return connector_status_disconnected; > > } > > -- > > 2.28.0 > >