Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210926202027.GA588220@bhelgaas> <70a472b8-7a3e-1792-efe4-125584231824@kylinos.cn>
In-Reply-To: <70a472b8-7a3e-1792-efe4-125584231824@kylinos.cn>
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
Date:   Mon, 27 Sep 2021 11:57:27 +0800
Message-ID: <CAAd53p6gfx-=x8aagdFUerz8wKKScLYPu+q96Q=g73RC-WXO5g@mail.gmail.com>
Subject: Re: [PATCH] PCI/sysfs: add write attribute for boot_vga
To:     =?UTF-8?B?5p2O55yf6IO9?= <lizhenneng@kylinos.cn>
Cc:     Bjorn Helgaas <helgaas@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Linux PCI <linux-pci@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Sep 27, 2021 at 11:46 AM =E6=9D=8E=E7=9C=9F=E8=83=BD <lizhenneng@ky=
linos.cn> wrote:
>
>
> =E5=9C=A8 2021/9/27 =E4=B8=8A=E5=8D=884:20, Bjorn Helgaas =E5=86=99=E9=81=
=93:
> > On Sun, Sep 26, 2021 at 03:15:39PM +0800, Zhenneng Li wrote:
> >> Add writing attribute for boot_vga sys node,
> >> so we can config default video display
> >> output dynamically when there are two video
> >> cards on a machine.
> >>
> >> Xorg server will determine running on which
> >> video card based on boot_vga node's value.
> > When you repost this, please take a look at the git commit log history
> > and make yours similar.  Specifically, the subject should start with a
> > capital letter, and the body should be rewrapped to fill 75
> > characters.
> >
> > Please contrast this with the existing VGA arbiter.  See
> > Documentation/gpu/vgaarbiter.rst.  It sounds like this may overlap
> > with the VGA arbiter functionality, so this should explain why we need
> > both and how they interact.
>
> "Some "legacy" VGA devices implemented on PCI typically have the same
> hard-decoded addresses as they did on ISA. When multiple PCI devices are
> accessed at same time they need some kind of coordination. ", this is
> the explain of config VGA_ARB, that is to say, some legacy vga devices
> need use the same pci bus address, if user app(such as xorg) want access
> card A, but card A and card B have same bus address,  then VGA
> agaarbiter will determine will card to be accessed.
>
> And xorg will read boot_vga to determine which graphics card is the
> primary graphics output device.
>
> That is the difference about boot_vga and vgaarbiter.

So does kernel select the wrong card for boot VGA?
Or is this a feature that to switch Xorg's graphics renderer?
From what you described, it seems to be the latter one, and I think it
should be implemented in Xorg.

Kai-Heng

>
>
>
> >
> >> Signed-off-by: Zhenneng Li <lizhenneng@kylinos.cn>
> >> ---
> >>   drivers/pci/pci-sysfs.c | 24 +++++++++++++++++++++++-
> >>   1 file changed, 23 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> >> index 7bbf2673c7f2..a6ba19ce7adb 100644
> >> --- a/drivers/pci/pci-sysfs.c
> >> +++ b/drivers/pci/pci-sysfs.c
> >> @@ -664,7 +664,29 @@ static ssize_t boot_vga_show(struct device *dev, =
struct device_attribute *attr,
> >>                        !!(pdev->resource[PCI_ROM_RESOURCE].flags &
> >>                           IORESOURCE_ROM_SHADOW));
> >>   }
> >> -static DEVICE_ATTR_RO(boot_vga);
> >> +
> >> +static ssize_t boot_vga_store(struct device *dev, struct device_attri=
bute *attr,
> >> +                          const char *buf, size_t count)
> >> +{
> >> +    unsigned long val;
> >> +    struct pci_dev *pdev =3D to_pci_dev(dev);
> >> +    struct pci_dev *vga_dev =3D vga_default_device();
> >> +
> >> +    if (kstrtoul(buf, 0, &val) < 0)
> >> +            return -EINVAL;
> >> +
> >> +    if (val !=3D 1)
> >> +            return -EINVAL;
> >> +
> >> +    if (!capable(CAP_SYS_ADMIN))
> >> +            return -EPERM;
> >> +
> >> +    if (pdev !=3D vga_dev)
> >> +            vga_set_default_device(pdev);
> >> +
> >> +    return count;
> >> +}
> >> +static DEVICE_ATTR_RW(boot_vga);
> >>
> >>   static ssize_t pci_read_config(struct file *filp, struct kobject *ko=
bj,
> >>                             struct bin_attribute *bin_attr, char *buf,
> >> --
> >> 2.25.1
> >>
> >>
> >> No virus found
> >>              Checked by Hillstone Network AntiVirus