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=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,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 6FBF0C4363A for ; Fri, 9 Oct 2020 14:19:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 821AA2231B for ; Fri, 9 Oct 2020 14:19:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="TXHPzxH9" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 821AA2231B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9297994000A; Fri, 9 Oct 2020 10:19:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D91C940007; Fri, 9 Oct 2020 10:19:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7C88194000A; Fri, 9 Oct 2020 10:19:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0227.hostedemail.com [216.40.44.227]) by kanga.kvack.org (Postfix) with ESMTP id 4B002940007 for ; Fri, 9 Oct 2020 10:19:07 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id DAB301EE6 for ; Fri, 9 Oct 2020 14:19:06 +0000 (UTC) X-FDA: 77352593892.30.fall53_3507d47271e1 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin30.hostedemail.com (Postfix) with ESMTP id ACE98180B3C8B for ; Fri, 9 Oct 2020 14:19:06 +0000 (UTC) X-HE-Tag: fall53_3507d47271e1 X-Filterd-Recvd-Size: 8116 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf27.hostedemail.com (Postfix) with ESMTP for ; Fri, 9 Oct 2020 14:19:05 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id l4so9113413ota.7 for ; Fri, 09 Oct 2020 07:19:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=MM70QuqTQsUBVq7gYXvXBZty2HeYwYo1AwlGqBndKzc=; b=TXHPzxH9Da/xVr/PCQoDGy8c0zPWy+ggD3CN9+uIIarhuboqtJeD+WQppuYxHsd6Lw Y7BHU63LYKcn8Q19C0ojPAIbHvLgUU7idHjBxl/cqvgWdtq9sYwq7aJadM3ewUTikvYS Or+ZtapYPyo5QGIWg5F96VAf0fQY9D81hLV+0= 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:content-transfer-encoding; bh=MM70QuqTQsUBVq7gYXvXBZty2HeYwYo1AwlGqBndKzc=; b=ZE4AA/5MoXGKfbfndzgldADqcBZKLlFfY7QRpKhBT6cHu0t62Kfvy7+KkNlGoTZWpv /nGTDuIrc+ZKG04ZDcyGQcSYaMEkPLUgGF9pIxjYWorNnplVd/+1wDmIYHkqST498SyY mid/tHvon/jUxNGdvyoQB3GutS55Hh+ub+KMPHvqaTawwGvAe17S15UShWV0e/clHHqD VnPIdyNn17zP3pLIyDrxR4j4sCY4hu5E7+OSfWMSlNG+A9txC9IEUkhb4KbsUDVr01Jo TfKAH8O8VxH3a73EOsjzB82lUPsWz83tKDETJW7Yt1rU8wEagYPFFobQL/gPnEZFmCLJ YGRw== X-Gm-Message-State: AOAM531/MuL0ubmVHAnU4f8d2A+/6EAeORh+bmY4ws0/W2OOpqAuGNYv v27amuiLVQIQFbKLBJ9ekuQau2U+3rHrsTOp4JohT0zBPbAG5Q== X-Google-Smtp-Source: ABdhPJzNidxPOYe2uXf9i2kyX3nO4EFb7XRxsxlaQUOxC+n0ejMSsQdbIKmUu2me6/FBsUnaPi9FuwTaAIvXBztd7mE= X-Received: by 2002:a05:6830:1c3c:: with SMTP id f28mr9534834ote.188.1602253144991; Fri, 09 Oct 2020 07:19:04 -0700 (PDT) MIME-Version: 1.0 References: <20201009075934.3509076-1-daniel.vetter@ffwll.ch> <20201009075934.3509076-18-daniel.vetter@ffwll.ch> <20201009094750.GQ6112@intel.com> <20201009104154.GR6112@intel.com> In-Reply-To: <20201009104154.GR6112@intel.com> From: Daniel Vetter Date: Fri, 9 Oct 2020 16:18:53 +0200 Message-ID: Subject: Re: [PATCH v2 17/17] drm/i915: Properly request PCI BARs To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: DRI Development , LKML , linux-s390 , linux-samsung-soc , Jan Kara , Kees Cook , KVM list , Jason Gunthorpe , Linux PCI , Linux MM , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Bjorn Helgaas , Daniel Vetter , Dan Williams , Andrew Morton , Linux ARM , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Oct 9, 2020 at 12:42 PM Ville Syrj=C3=A4l=C3=A4 wrote: > > On Fri, Oct 09, 2020 at 12:01:39PM +0200, Daniel Vetter wrote: > > On Fri, Oct 9, 2020 at 11:47 AM Ville Syrj=C3=A4l=C3=A4 > > wrote: > > > > > > On Fri, Oct 09, 2020 at 09:59:34AM +0200, Daniel Vetter wrote: > > > > When trying to test my CONFIG_IO_STRICT_DEVMEM changes I realized t= hey > > > > do nothing for i915. Because i915 doesn't request any regions, like > > > > pretty much all drm pci drivers. I guess this is some very old > > > > remnants from the userspace modesetting days, when we wanted to > > > > co-exist with the fbdev driver. Which usually requested these > > > > resources. > > > > > > > > But makes me wonder why the pci subsystem doesn't just request > > > > resource automatically when we map a bar and a pci driver is bound? > > > > > > > > Knowledge about which pci bars we need kludged together from > > > > intel_uncore.c and intel_gtt.c from i915 and intel-gtt.c over in th= e > > > > fake agp driver. > > > > > > > > Signed-off-by: Daniel Vetter > > > > Cc: Jason Gunthorpe > > > > Cc: Kees Cook > > > > Cc: Dan Williams > > > > Cc: Andrew Morton > > > > Cc: John Hubbard > > > > Cc: J=C3=A9r=C3=B4me Glisse > > > > Cc: Jan Kara > > > > Cc: Dan Williams > > > > Cc: linux-mm@kvack.org > > > > Cc: linux-arm-kernel@lists.infradead.org > > > > Cc: linux-samsung-soc@vger.kernel.org > > > > Cc: linux-media@vger.kernel.org > > > > Cc: Bjorn Helgaas > > > > Cc: linux-pci@vger.kernel.org > > > > --- > > > > drivers/gpu/drm/i915/intel_uncore.c | 25 +++++++++++++++++++++++-- > > > > 1 file changed, 23 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/= i915/intel_uncore.c > > > > index 54e201fdeba4..ce39049d8919 100644 > > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > > @@ -1692,10 +1692,13 @@ static int uncore_mmio_setup(struct intel_u= ncore *uncore) > > > > struct pci_dev *pdev =3D i915->drm.pdev; > > > > int mmio_bar; > > > > int mmio_size; > > > > + int bar_selection; > > > > > > Signed bitmasks always make me uneasy. But looks like > > > that's what it is in the pci api. So meh. > > > > Yeah it's surprising. > > > > > > + int ret; > > > > > > > > mmio_bar =3D IS_GEN(i915, 2) ? 1 : 0; > > > > + bar_selection =3D BIT (2) | BIT(mmio_bar); > > > ^ > > > spurious space > > > > > > That's also not correct for gen2 I think. > > > > > > gen2: > > > 0 =3D GMADR > > > 1 =3D MMADR > > > 2 =3D IOBAR > > > > > > gen3: > > > 0 =3D MMADR > > > 1 =3D IOBAR > > > 2 =3D GMADR > > > 3 =3D GTTADR > > > > > > gen4+: > > > 0+1 =3D GTTMMADR > > > 2+3 =3D GMADR > > > 4 =3D IOBAR > > > > > > Maybe we should just have an explicit list of bars like that in a > > > comment? > > > > > > I'd also suggest sucking this bitmask calculation into a small helper > > > so you can reuse it for the release. > > > > tbh I just hacked this up for testing. Given how almost no other drm > > driver does this, I'm wondering whether we should or not. > > > > Also the only reason why I didn't just use the pci_request_regions > > helper is to avoid the vga ioport range, since that's managed by > > vgaarbiter. > > VGA io range isn't part of any bar. Or do you mean just the io decode > enable bit in the pci command register? That should be just a matter > or pci_enable_device() vs. pci_enable_device_mem() I think. So nothing > to do with which bars we've requested IIRC. > > > > > So I think if we go for this for real we should: > > - register the vga ioport range in the vgaarbiter > > - have a pci_request_iomem_regions helper that grabs all mem bars > > - roll that out to all drm pci drivers > > > > Or something like that. The other complication is when we resize the > > iobar. So not really sure what to do here. > > We resize it? By default I thought firmware puts everything (well, squeezes) into the lower 32bit. Maybe they stopped doing that. So when we want the full bar (for discrete at least) we need to resize it and put it somewhere in the 64bit range above end of system memory. So I guess correct phrasing is "we will resize it" :-) -Daniel --=20 Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch