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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B7A6AC433EF for ; Fri, 3 Jun 2022 23:21:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343816AbiFCXV0 (ORCPT ); Fri, 3 Jun 2022 19:21:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236404AbiFCXVZ (ORCPT ); Fri, 3 Jun 2022 19:21:25 -0400 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2AD413CD3 for ; Fri, 3 Jun 2022 16:21:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1654298484; x=1685834484; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=bUMNBuUXY/UAvVKqEyweKMXGBGukeILdGV/BgiEzxl4=; b=imhfqCW4Xcp/3VFu3P0HRxdwyIsLG4o8o/d4uQSNfKF7PWRD/tTE6NOU Tci3IwoTEhlEn9nf1VvuHGT2QfTm/o4aTKaYdxEya2l0fDJol0U2q1A4T M1P17MIiSVfk9OLFAbzegRXFw3N0z/dTCvcal8lbKdAjt96WawpmeLSNl g=; Received: from unknown (HELO ironmsg03-sd.qualcomm.com) ([10.53.140.143]) by alexa-out-sd-01.qualcomm.com with ESMTP; 03 Jun 2022 16:21:24 -0700 X-QCInternal: smtphost Received: from nasanex01b.na.qualcomm.com ([10.46.141.250]) by ironmsg03-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jun 2022 16:21:24 -0700 Received: from [10.71.110.111] (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Fri, 3 Jun 2022 16:21:23 -0700 Message-ID: Date: Fri, 3 Jun 2022 16:21:23 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [Freedreno] [PATCH 3/3] drm/msm/dpu: Add interface support for CRC debugfs Content-Language: en-US To: Dmitry Baryshkov CC: , , , , , , , References: <20220527185407.162-1-quic_jesszhan@quicinc.com> <20220527185407.162-4-quic_jesszhan@quicinc.com> <266fdac1-db57-a729-3d73-42d2b34017cd@linaro.org> From: Jessica Zhang In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01b.na.qualcomm.com (10.46.141.250) Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 6/3/2022 12:02 AM, Dmitry Baryshkov wrote: > On Fri, 3 Jun 2022 at 04:02, Jessica Zhang wrote: >> On 6/2/2022 3:51 PM, Dmitry Baryshkov wrote: >>> On 28/05/2022 01:23, Jessica Zhang wrote: >>>> On 5/27/2022 12:46 PM, Dmitry Baryshkov wrote: >>>>> On 27/05/2022 21:54, Jessica Zhang wrote: >>>>>> Add support for writing CRC values for the interface block to >>>>>> the debugfs by calling the necessary MISR setup/collect methods. >>>>>> >>>>>> Signed-off-by: Jessica Zhang > > [skipped] > >>>>>> + >>>>>> + phys->hw_intf->ops.setup_misr(phys->hw_intf, true, 1); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +int dpu_encoder_get_crc(const struct drm_encoder *drm_enc) >>>>>> +{ >>>>>> + struct dpu_encoder_virt *dpu_enc; >>>>>> + u32 crcs[MAX_PHYS_ENCODERS_PER_VIRTUAL]; >>>>>> + >>>>>> + int i, rc; >>>>>> + >>>>>> + if (!drm_enc->crtc) { >>>>>> + DRM_ERROR("no crtc found for encoder %d\n", drm_enc->index); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + >>>>>> + dpu_enc = to_dpu_encoder_virt(drm_enc); >>>>>> + >>>>>> + for (i = 0; i < dpu_enc->num_phys_encs; i++) { >>>>>> + struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>>>>> + >>>>>> + if (!phys->hw_intf || !phys->hw_intf->ops.collect_misr) >>>>>> + continue; >>>>>> + >>>>>> + rc = phys->hw_intf->ops.collect_misr(phys->hw_intf, &crcs[i]); >>>>> >>>>> This doesn't look fully correct. Do we need to skip the indices for >>>>> the phys without a backing hw_intf? >>>> >>>> Sorry if I'm misunderstanding your question, but don't we need to have >>>> a backing hw_intf (and skip if there isn't any) since the methods for >>>> collecting/setting MISR registers is within the hw_intf? >>> >>> Yes. So the question if we should skip the phys and leave the crcs[i] >>> untouched, skip the phys and sset crcs[i] to 0 or change >>> dpu_crtc_parse_crc_source() to return the number of intf-backed >>> phys_enc's and do not skip any crcs[i]. >> >> Thanks for the clarification. >> >> Is it possible to hit a case where a phys_encoder won't have a >> corresponding hw_intf? >> >> AFAIK, it seems guaranteed that a phys_encoder will have an hw_intf >> since dpu_encoder_setup_display will skip incrementing num_phys_encs if >> dpu_encoder_get_intf fails [1]. > > WB encoders won't have hw_intf. The code checks that either get_intf > or get_wb succeeds. Got it, I see your point. I'll change the values_cnt to only include the intf-backed phys_encoders then. Thanks, Jessica Zhang > >> >> [1] >> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2263 > > > -- > With best wishes > Dmitry 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 89765C43334 for ; Fri, 3 Jun 2022 23:21:26 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9901F112442; Fri, 3 Jun 2022 23:21:25 +0000 (UTC) Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by gabe.freedesktop.org (Postfix) with ESMTPS id 14973112442; Fri, 3 Jun 2022 23:21:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1654298485; x=1685834485; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=bUMNBuUXY/UAvVKqEyweKMXGBGukeILdGV/BgiEzxl4=; b=R+OmJmUQWYW9uyAlBgeqbVKsGBSWBFjR+wznpfRq6MpyuOl/BuBuKIcA 7ov/qJhbaw/FkjmDiwghxWNOpgFLKLHNTQAdDaPi7AMvy9c4NOKf1fVGA YI9rxTDybVKvrmqN6PZgpdQtj5qBArbMphC8s2crwhc1nYLxG5yOSzcWR Y=; Received: from unknown (HELO ironmsg03-sd.qualcomm.com) ([10.53.140.143]) by alexa-out-sd-01.qualcomm.com with ESMTP; 03 Jun 2022 16:21:24 -0700 X-QCInternal: smtphost Received: from nasanex01b.na.qualcomm.com ([10.46.141.250]) by ironmsg03-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jun 2022 16:21:24 -0700 Received: from [10.71.110.111] (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Fri, 3 Jun 2022 16:21:23 -0700 Message-ID: Date: Fri, 3 Jun 2022 16:21:23 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [Freedreno] [PATCH 3/3] drm/msm/dpu: Add interface support for CRC debugfs Content-Language: en-US To: Dmitry Baryshkov References: <20220527185407.162-1-quic_jesszhan@quicinc.com> <20220527185407.162-4-quic_jesszhan@quicinc.com> <266fdac1-db57-a729-3d73-42d2b34017cd@linaro.org> From: Jessica Zhang In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01b.na.qualcomm.com (10.46.141.250) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-msm@vger.kernel.org, quic_abhinavk@quicinc.com, dri-devel@lists.freedesktop.org, swboyd@chromium.org, seanpaul@chromium.org, quic_aravindh@quicinc.com, freedreno@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 6/3/2022 12:02 AM, Dmitry Baryshkov wrote: > On Fri, 3 Jun 2022 at 04:02, Jessica Zhang wrote: >> On 6/2/2022 3:51 PM, Dmitry Baryshkov wrote: >>> On 28/05/2022 01:23, Jessica Zhang wrote: >>>> On 5/27/2022 12:46 PM, Dmitry Baryshkov wrote: >>>>> On 27/05/2022 21:54, Jessica Zhang wrote: >>>>>> Add support for writing CRC values for the interface block to >>>>>> the debugfs by calling the necessary MISR setup/collect methods. >>>>>> >>>>>> Signed-off-by: Jessica Zhang > > [skipped] > >>>>>> + >>>>>> + phys->hw_intf->ops.setup_misr(phys->hw_intf, true, 1); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +int dpu_encoder_get_crc(const struct drm_encoder *drm_enc) >>>>>> +{ >>>>>> + struct dpu_encoder_virt *dpu_enc; >>>>>> + u32 crcs[MAX_PHYS_ENCODERS_PER_VIRTUAL]; >>>>>> + >>>>>> + int i, rc; >>>>>> + >>>>>> + if (!drm_enc->crtc) { >>>>>> + DRM_ERROR("no crtc found for encoder %d\n", drm_enc->index); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + >>>>>> + dpu_enc = to_dpu_encoder_virt(drm_enc); >>>>>> + >>>>>> + for (i = 0; i < dpu_enc->num_phys_encs; i++) { >>>>>> + struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>>>>> + >>>>>> + if (!phys->hw_intf || !phys->hw_intf->ops.collect_misr) >>>>>> + continue; >>>>>> + >>>>>> + rc = phys->hw_intf->ops.collect_misr(phys->hw_intf, &crcs[i]); >>>>> >>>>> This doesn't look fully correct. Do we need to skip the indices for >>>>> the phys without a backing hw_intf? >>>> >>>> Sorry if I'm misunderstanding your question, but don't we need to have >>>> a backing hw_intf (and skip if there isn't any) since the methods for >>>> collecting/setting MISR registers is within the hw_intf? >>> >>> Yes. So the question if we should skip the phys and leave the crcs[i] >>> untouched, skip the phys and sset crcs[i] to 0 or change >>> dpu_crtc_parse_crc_source() to return the number of intf-backed >>> phys_enc's and do not skip any crcs[i]. >> >> Thanks for the clarification. >> >> Is it possible to hit a case where a phys_encoder won't have a >> corresponding hw_intf? >> >> AFAIK, it seems guaranteed that a phys_encoder will have an hw_intf >> since dpu_encoder_setup_display will skip incrementing num_phys_encs if >> dpu_encoder_get_intf fails [1]. > > WB encoders won't have hw_intf. The code checks that either get_intf > or get_wb succeeds. Got it, I see your point. I'll change the values_cnt to only include the intf-backed phys_encoders then. Thanks, Jessica Zhang > >> >> [1] >> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2263 > > > -- > With best wishes > Dmitry