Message ID | 20221114194133.1535178-1-robdclark@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2330250wru; Mon, 14 Nov 2022 11:43:39 -0800 (PST) X-Google-Smtp-Source: AA0mqf7+2Eak7N5DMO4eJrGtO7RyWNl39tLtTWeaM3BphKJqUmq6IG8HOQks17ZSLdyHsahDpjA2 X-Received: by 2002:a17:907:bc6:b0:7ae:1e53:95b2 with SMTP id ez6-20020a1709070bc600b007ae1e5395b2mr11001544ejc.333.1668455019455; Mon, 14 Nov 2022 11:43:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668455019; cv=none; d=google.com; s=arc-20160816; b=oXeyNMv/BFJ2Te/jIjuf79gwoO7pJn+cTQyuV2jijwT3gi1BSZfN0bk3Ud187IYZo6 Abn9GaR+00PlJC7YPCjhhrZ5rPt5krLsAWeoPu3pUkiS4AOxgA7YoX9Lz7rHCG5HL7Q9 EWe7BtwNXMavdrBVu9/1q9nvhaSYM67fGP60a7ZUfdOSWokwjNFcWD5KTipBM5ztlpUt y4gErcY6JXyzqdngdV4A7t7VQmO+b4DY4lksyTEeNXSjSj2CNzdXzvXlxWp0925NkmL1 Cw2lIa2ABuFfDoaPwWpzl2Kyk1HIhaCq0nIwXcVnidCN/4ZHNGlJi4w3vyH3yAKqsQUC 7wMQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=3XrI4nxvGHUCxoc8HuMbGJ+vNpFCW7F3PwRmwEeEnBE=; b=fQDFbiaQA1Mcet6W5rCEaW5CdI8m5ImsBPF7hQGHyzY1hXT6APte34UL4tP7TG5eU+ EqSBC3c529N6AQZZs0QL2ORSh56RtDQoDA/XJU/X+BuUDzozoNHKi8LY+4FDVPS9K9IY 049O77GVzHSYs/HkkxM3RbJdFw9mcpAKiidRtjXr0L36xmXAk0zygzL5PR203TsGc4W5 J6z0NpG33v4syWHOdLM1r4WAUH8d3Uk0rU9O6ie/m4Rps0GIVCh1tlfmL2cLVhk+rFEs B3Sk3nMEnsHIVbJs9HnM6t1YZOoEXiOX+1OnKyZ4qPcFeygoxen2KsTRlP5+CJbqcICk e9rQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=gppJTB4T; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cw12-20020a170906478c00b007804b5a2c48si9611511ejc.521.2022.11.14.11.43.14; Mon, 14 Nov 2022 11:43:39 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=gppJTB4T; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237077AbiKNTlP (ORCPT <rfc822;zwp10758@gmail.com> + 99 others); Mon, 14 Nov 2022 14:41:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229484AbiKNTlL (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 14 Nov 2022 14:41:11 -0500 Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9BEE01A38B; Mon, 14 Nov 2022 11:41:10 -0800 (PST) Received: by mail-pf1-x42e.google.com with SMTP id q9so12007303pfg.5; Mon, 14 Nov 2022 11:41:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=3XrI4nxvGHUCxoc8HuMbGJ+vNpFCW7F3PwRmwEeEnBE=; b=gppJTB4ThxkXJZLbWfSW4pZ121wwq072YhQUh+SzfkcvR94GOYRlkaQZg7dO7+yQNN 5k3gNK8L1xS3vged5EKsB/9a71x2UreuBfs7GFBWV2mE+xY9TTS16jvL7ki0RpsXfaJV Fn3lgL3+mHi9IE5wNArMI0w7WgUEH2JE4OjytDywiWxE+1stkdv2Cul5CrgXn6Szpt6z 7jarednJ9M0GUC0dprQE4/2rcI04RCTEcgUkUfki2i9QZPbhAxvP1RrdSiIWznaI7VRa 0N4RaT5hz1BpQw8R4j91f9AxlSovfy1IUhd/99Xnb4TxVGlPSo9zlNgObRNYNcjTmOyA pDxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=3XrI4nxvGHUCxoc8HuMbGJ+vNpFCW7F3PwRmwEeEnBE=; b=x4+SM8CKue60fFAzDrRnAKQS43R9ZiDi/Pa5LX44sxsY+3cDpvKnXI+I8mLSBKDan6 sIsfssffL+K0kpazCztnqWXrCUYN0g1pxSL0JkW+8GTfn5VUTvMgpIR4u431o18qxqQw ffdQ0tqomTi9izv1i+iLjW7EIYqt23Y3RzN6BxWeax414pndx2XQQyG9+Nab/n8Px0tJ z/sQafdMdmw+4CFZtEANwVMOYWjwQWbvBqGKORXWJePOWWYdEQuow9KTNDeo5KX4QrS3 Kj9VHlhpLI0i5GAF/GXRCuW5ZVUApw9+EoGwFRgQaz31iOcb63gMZHWAcKk0WF+hVDD6 BwvQ== X-Gm-Message-State: ANoB5pl3eYvIp036UFz7/6Uoq4kOUwQfP50OI88FhLZ8EXaVhb0gh8BM 25NLkcDARkRfGkuAlFoIusI= X-Received: by 2002:a63:5206:0:b0:41a:5a80:5f20 with SMTP id g6-20020a635206000000b0041a5a805f20mr12701044pgb.442.1668454870081; Mon, 14 Nov 2022 11:41:10 -0800 (PST) Received: from localhost ([2a00:79e1:abd:4a00:2703:3c72:eb1a:cffd]) by smtp.gmail.com with ESMTPSA id gk9-20020a17090b118900b00205db4ff6dfsm6841481pjb.46.2022.11.14.11.41.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 11:41:09 -0800 (PST) From: Rob Clark <robdclark@gmail.com> To: dri-devel@lists.freedesktop.org Cc: freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, Rob Clark <robdclark@chromium.org>, Rob Clark <robdclark@gmail.com>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Akhil P Oommen <quic_akhilpo@quicinc.com>, Chia-I Wu <olvaffe@gmail.com>, Konrad Dybcio <konrad.dybcio@somainline.org>, Douglas Anderson <dianders@chromium.org>, linux-kernel@vger.kernel.org (open list) Subject: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer Date: Mon, 14 Nov 2022 11:41:31 -0800 Message-Id: <20221114194133.1535178-1-robdclark@gmail.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749501890176823742?= X-GMAIL-MSGID: =?utf-8?q?1749501890176823742?= |
Series |
drm/msm/a6xx: Fix speed-bin detection vs probe-defer
|
|
Commit Message
Rob Clark
Nov. 14, 2022, 7:41 p.m. UTC
From: Rob Clark <robdclark@chromium.org> If we get an error (other than -ENOENT) we need to propagate that up the stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with whatever OPP(s) are represented by bit zero. Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu") Signed-off-by: Rob Clark <robdclark@chromium.org> --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 11/15/2022 1:11 AM, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > If we get an error (other than -ENOENT) we need to propagate that up the > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > whatever OPP(s) are represented by bit zero. > > Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu") > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 7fe60c65a1eb..96de2202c86c 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) > DRM_DEV_ERROR(dev, > "failed to read speed-bin (%d). Some OPPs may not be supported by hardware", I just noticed and was going to send a similar fix. We should remove ". Some OPPs may not be supported by hardware" here. Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com> Btw, on msm-next-external-fixes + this fix, I still see boot up issue in herobrine due to drm_dev_alloc() failure with -ENOSPC error. -Akhil. > ret); > - goto done; > + return ret; > } > > supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
Hi, On Mon, Nov 14, 2022 at 11:41 AM Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > If we get an error (other than -ENOENT) we need to propagate that up the > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > whatever OPP(s) are represented by bit zero. Can you explain the "whatever OPP(s) are represented by bit zero" part? This doesn't seem to be true because `supp_hw` is initiated to UINT_MAX. If I'm remembering how this all works, doesn't that mean that if we get an error we'll assume all OPPs are OK? I'm not saying that I'm against your change, but I think maybe you're misdescribing the old behavior. Speaking of the initialization of supp_hw, if we want to change the behavior like your patch does then we should be able to remove that initialization, right? I would also suspect that your patch will result in a compiler warning, at least on some compilers. The goto label `done` is no longer needed, right? -Doug
On 11/15/2022 1:57 AM, Doug Anderson wrote: > Hi, > > On Mon, Nov 14, 2022 at 11:41 AM Rob Clark <robdclark@gmail.com> wrote: >> From: Rob Clark <robdclark@chromium.org> >> >> If we get an error (other than -ENOENT) we need to propagate that up the >> stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with >> whatever OPP(s) are represented by bit zero. > Can you explain the "whatever OPP(s) are represented by bit zero" > part? This doesn't seem to be true because `supp_hw` is initiated to > UINT_MAX. If I'm remembering how this all works, doesn't that mean > that if we get an error we'll assume all OPPs are OK? > > I'm not saying that I'm against your change, but I think maybe you're > misdescribing the old behavior. > > Speaking of the initialization of supp_hw, if we want to change the > behavior like your patch does then we should be able to remove that > initialization, right? > > I would also suspect that your patch will result in a compiler > warning, at least on some compilers. The goto label `done` is no > longer needed, right? > > -Doug You are right about the commit message. The problem is we can't enable all bits in supp_hw anymore due to changes like this: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220829011035.1.Ie3564662150e038571b7e2779cac7229191cf3bf@changeid/ This creates 2 opps with same freq when supp_hw = UINT_MAX. -Akhil.
On Mon, Nov 14, 2022 at 12:27 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Mon, Nov 14, 2022 at 11:41 AM Rob Clark <robdclark@gmail.com> wrote: > > > > From: Rob Clark <robdclark@chromium.org> > > > > If we get an error (other than -ENOENT) we need to propagate that up the > > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > > whatever OPP(s) are represented by bit zero. > > Can you explain the "whatever OPP(s) are represented by bit zero" > part? This doesn't seem to be true because `supp_hw` is initiated to > UINT_MAX. If I'm remembering how this all works, doesn't that mean > that if we get an error we'll assume all OPPs are OK? Oh, that's right.. and even worse! Ok, stand by for v2 > I'm not saying that I'm against your change, but I think maybe you're > misdescribing the old behavior. > > Speaking of the initialization of supp_hw, if we want to change the > behavior like your patch does then we should be able to remove that > initialization, right? > > I would also suspect that your patch will result in a compiler > warning, at least on some compilers. The goto label `done` is no > longer needed, right? > > -Doug
On Mon, Nov 14, 2022 at 11:59 AM Akhil P Oommen <quic_akhilpo@quicinc.com> wrote: > > On 11/15/2022 1:11 AM, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > If we get an error (other than -ENOENT) we need to propagate that up the > > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > > whatever OPP(s) are represented by bit zero. > > > > Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu") > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > index 7fe60c65a1eb..96de2202c86c 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > @@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) > > DRM_DEV_ERROR(dev, > > "failed to read speed-bin (%d). Some OPPs may not be supported by hardware", > I just noticed and was going to send a similar fix. We should remove ". > Some OPPs may not be supported by hardware" here. > > Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > > Btw, on msm-next-external-fixes + this fix, I still see boot up issue > in herobrine due to drm_dev_alloc() failure with -ENOSPC error. Could you track it down one level deeper? I wonder if there is some missing cleanup in the probe-defer path and we end up failing in drm_minor_alloc() or something along those lines BR, -R > -Akhil. > > ret); > > - goto done; > > + return ret; > > } > > > > supp_hw = fuse_to_supp_hw(dev, rev, speedbin); >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 7fe60c65a1eb..96de2202c86c 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) DRM_DEV_ERROR(dev, "failed to read speed-bin (%d). Some OPPs may not be supported by hardware", ret); - goto done; + return ret; } supp_hw = fuse_to_supp_hw(dev, rev, speedbin);