From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH 3/3] ARM64: DT: add iommu for msm8916 Date: Wed, 31 May 2017 09:14:25 -0700 Message-ID: <20170531161425.GY20170@codeaurora.org> References: <20170525174822.27996-1-robdclark@gmail.com> <20170525174822.27996-4-robdclark@gmail.com> <20170531001435.GR20170@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:41984 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751000AbdEaQOd (ORCPT ); Wed, 31 May 2017 12:14:33 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Rob Clark Cc: linux-arm-msm , Andy Gross , Stanimir Varbanov On 05/31, Rob Clark wrote: > On Tue, May 30, 2017 at 8:14 PM, Stephen Boyd wrote: > > On 05/25, Rob Clark wrote: > >> + apps_iommu: iommu@1ef0000 { > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + #iommu-cells = <1>; > >> + compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1"; > >> + ranges = <0 0x1e20000 0x40000>; > >> + reg = <0x1ef0000 0x3000>; > >> + clocks = <&gcc GCC_SMMU_CFG_CLK>, > >> + <&gcc GCC_APSS_TCU_CLK>; > >> + clock-names = "iface", "bus"; > >> + qcom,iommu-secure-id = <17>; > >> + > >> + // mdp_0: > >> + iommu-ctx@4000 { > >> + compatible = "qcom,msm-iommu-v1-ns"; > >> + reg = <0x4000 0x1000>; > >> + interrupts = ; > > > > s/0/IRQ_TYPE_LEVEL_HIGH/ > > 0 actually seems to be _NONE.. so using _HIGH would change how irq is > configured (according to gic_configure_irq()). Do you expect that to > work? I'm probably going based on what was in downstream dt, and some > of that is more a pain to retest so I'd rather not change the type > unless at least one of us knows what they are doing. I believe the default setting for all these interrupts are IRQ_TYPE_LEVEL_HIGH already, so there shouldn't be any change to behavior and things should still work. Would be worth double checking what the GIC has it configured as with IRQ_TYPE_NONE here by looking at /proc/interrupts. I suspect it's level high. > > >> + }; > >> + > >> + // venus_ns: > >> + iommu-ctx@5000 { > >> + compatible = "qcom,msm-iommu-v1-sec"; > >> + reg = <0x5000 0x1000>; > >> + interrupts = ; > > > > s/0/IRQ_TYPE_LEVEL_HIGH/ > > > > Is it the same interrupt number (70) twice? Not 71 or something? > > According to downstream. Not *entirely* sure how that is supposed to > work as far as dispatching fault callback to the right driver, but at > least unlike with the gpu, if we get a fault for mdp, that is entirely > the kernels fault. So meh? > > If you know better, let me know. > Ok, I see what's going on. This design still seems odd to me. We have an interrupt per-context bank in DT so that we can map the top-level aggregated context bank interrupt (70) to each context bank that has their interrupt routed there. Depending on the bootloader configuration we could have many context banks route their fault interrupt to the same interrupt at the GIC or they could go to different top-level interrupts. In the GFX IOMMU case, hw folks decided to _not_ aggregate context bank interrupts at all so each context bank has a dedicated interrupt at the GIC. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project