diff mbox

Fix intel_detect_pch() to work in xen environment.

Message ID CAKhsbWZVAVqgzQLkJ0=SZHP-D35acPaahB9yAmNAEc9bpp8wdg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

G.R. June 18, 2013, 12:54 p.m. UTC
On Sun, Dec 23, 2012 at 2:51 PM, G.R. <firemeteor@users.sourceforge.net> wrote:
>>> Hi Jesse, I think I need to resend the patch with proper comment to
>>> have it formally accepted.
>>> Any guide line for formal patch submission? Do I need to start a
>>> separate thread?
>>
>> No, just cc Daniel Vetter.
>>
>> --
>> Jesse Barnes, Intel Open Source Technology Center
>
> Thanks,
> Resend with updated patch && Daniel involved.
> Also include the background info for easy reading.
>
> In XEN HVM guest, there is always an emulated PIIX3 ISA bridge on slot 01.0.
> This shadows the PCH ISA bridge on 1f.0 with the current
> intel_detect_pch() implementation.
> The issue can be easily solved by looping through all the ISA bridges
> until the first match is found, instead of just check against the
> first one.
>

This seems to have lost in the list. Retry it again with latest
torvalds' kernel:

Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Comments

Daniel Vetter June 19, 2013, 6:01 a.m. UTC | #1
Can you please resubmit this as a proper patch with a committ message,
signed-off-by line and everything else (see
Documentation/SubmittingPatches).
-Daniel

On Tue, Jun 18, 2013 at 2:54 PM, G.R. <firemeteor@users.sourceforge.net> wrote:
> On Sun, Dec 23, 2012 at 2:51 PM, G.R. <firemeteor@users.sourceforge.net> wrote:
>>>> Hi Jesse, I think I need to resend the patch with proper comment to
>>>> have it formally accepted.
>>>> Any guide line for formal patch submission? Do I need to start a
>>>> separate thread?
>>>
>>> No, just cc Daniel Vetter.
>>>
>>> --
>>> Jesse Barnes, Intel Open Source Technology Center
>>
>> Thanks,
>> Resend with updated patch && Daniel involved.
>> Also include the background info for easy reading.
>>
>> In XEN HVM guest, there is always an emulated PIIX3 ISA bridge on slot 01.0.
>> This shadows the PCH ISA bridge on 1f.0 with the current
>> intel_detect_pch() implementation.
>> The issue can be easily solved by looping through all the ISA bridges
>> until the first match is found, instead of just check against the
>> first one.
>>
>
> This seems to have lost in the list. Retry it again with latest
> torvalds' kernel:
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a2e4953..e38fa86 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -454,11 +454,18 @@ void intel_detect_pch(struct drm_device *dev)
>          * make graphics device passthrough work easy for VMM, that only
>          * need to expose ISA bridge to let driver know the real hardware
>          * underneath. This is a requirement from virtualization team.
> +        *
> +        * In some virtualized environments (e.g. XEN), there is irrelevant
> +        * ISA bridge in the system. To work reliably, we should scan trhough
> +        * all the ISA bridge devices and check for the first match, instead
> +        * of only checking the first one.
>          */
>         pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
> -       if (pch) {
> +       while (pch) {
> +               struct pci_dev * curr = pch;
>                 if (pch->vendor == PCI_VENDOR_ID_INTEL) {
>                         unsigned short id;
> +                       unsigned found = 1;
>                         id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
>                         dev_priv->pch_id = id;
>
> @@ -490,10 +497,20 @@ void intel_detect_pch(struct drm_device *dev)
>                                 DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
>                                 WARN_ON(!IS_HASWELL(dev));
>                                 WARN_ON(!IS_ULT(dev));
> +                       } else {
> +                               found = 0;
> +                       }
> +                       if (found) {
> +                               BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
> +                               pci_dev_put(pch);
> +                               break;
>                         }
> -                       BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
>                 }
> -               pci_dev_put(pch);
> +               pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
> +               pci_dev_put(curr);
> +       }
> +       if (!pch) {
> +               DRM_DEBUG_KMS("No PCH found?\n");
>         }
>  }
Ville Syrjälä June 19, 2013, 7:57 a.m. UTC | #2
On Tue, Jun 18, 2013 at 08:54:35PM +0800, G.R. wrote:
> On Sun, Dec 23, 2012 at 2:51 PM, G.R. <firemeteor@users.sourceforge.net> wrote:
> >>> Hi Jesse, I think I need to resend the patch with proper comment to
> >>> have it formally accepted.
> >>> Any guide line for formal patch submission? Do I need to start a
> >>> separate thread?
> >>
> >> No, just cc Daniel Vetter.
> >>
> >> --
> >> Jesse Barnes, Intel Open Source Technology Center
> >
> > Thanks,
> > Resend with updated patch && Daniel involved.
> > Also include the background info for easy reading.
> >
> > In XEN HVM guest, there is always an emulated PIIX3 ISA bridge on slot 01.0.
> > This shadows the PCH ISA bridge on 1f.0 with the current
> > intel_detect_pch() implementation.
> > The issue can be easily solved by looping through all the ISA bridges
> > until the first match is found, instead of just check against the
> > first one.
> >
> 
> This seems to have lost in the list. Retry it again with latest
> torvalds' kernel:

> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a2e4953..e38fa86 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -454,11 +454,18 @@ void intel_detect_pch(struct drm_device *dev)
>          * make graphics device passthrough work easy for VMM, that only
>          * need to expose ISA bridge to let driver know the real hardware
>          * underneath. This is a requirement from virtualization team.
> +        *
> +        * In some virtualized environments (e.g. XEN), there is irrelevant
> +        * ISA bridge in the system. To work reliably, we should scan trhough
> +        * all the ISA bridge devices and check for the first match, instead
> +        * of only checking the first one.
>          */
>         pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
> -       if (pch) {
> +       while (pch) {
> +               struct pci_dev * curr = pch;
                                  ^

No space there. I think checkpatch should catch that.

>                 if (pch->vendor == PCI_VENDOR_ID_INTEL) {
>                         unsigned short id;
> +                       unsigned found = 1;

bool found = true;

Although it would be easy to adjust the patch to not need a 'found'
variable.

>                         id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
>                         dev_priv->pch_id = id;
> 
> @@ -490,10 +497,20 @@ void intel_detect_pch(struct drm_device *dev)
>                                 DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
>                                 WARN_ON(!IS_HASWELL(dev));
>                                 WARN_ON(!IS_ULT(dev));
> +                       } else {
> +                               found = 0;
> +                       }
> +                       if (found) {
> +                               BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
> +                               pci_dev_put(pch);
> +                               break;
>                         }
> -                       BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
>                 }
> -               pci_dev_put(pch);
> +               pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
> +               pci_dev_put(curr);
> +       }
> +       if (!pch) {
> +               DRM_DEBUG_KMS("No PCH found?\n");
>         }
>  }
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
G.R. June 19, 2013, 3:29 p.m. UTC | #3
On Wed, Jun 19, 2013 at 2:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> Can you please resubmit this as a proper patch with a committ message,
> signed-off-by line and everything else (see
> Documentation/SubmittingPatches).
> -Daniel
>

Done, sent in a separate thread:
http://lists.freedesktop.org/archives/intel-gfx/2013-June/029161.html
Daniel Vetter June 20, 2013, 12:01 p.m. UTC | #4
On Wed, Jun 19, 2013 at 11:29:21PM +0800, G.R. wrote:
> On Wed, Jun 19, 2013 at 2:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > Can you please resubmit this as a proper patch with a committ message,
> > signed-off-by line and everything else (see
> > Documentation/SubmittingPatches).
> > -Daniel
>
> Done, sent in a separate thread:
> http://lists.freedesktop.org/archives/intel-gfx/2013-June/029161.html

Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a2e4953..e38fa86 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -454,11 +454,18 @@  void intel_detect_pch(struct drm_device *dev)
         * make graphics device passthrough work easy for VMM, that only
         * need to expose ISA bridge to let driver know the real hardware
         * underneath. This is a requirement from virtualization team.
+        *
+        * In some virtualized environments (e.g. XEN), there is irrelevant
+        * ISA bridge in the system. To work reliably, we should scan trhough
+        * all the ISA bridge devices and check for the first match, instead
+        * of only checking the first one.
         */
        pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
-       if (pch) {
+       while (pch) {
+               struct pci_dev * curr = pch;
                if (pch->vendor == PCI_VENDOR_ID_INTEL) {
                        unsigned short id;
+                       unsigned found = 1;
                        id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
                        dev_priv->pch_id = id;

@@ -490,10 +497,20 @@  void intel_detect_pch(struct drm_device *dev)
                                DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
                                WARN_ON(!IS_HASWELL(dev));
                                WARN_ON(!IS_ULT(dev));
+                       } else {
+                               found = 0;
+                       }
+                       if (found) {
+                               BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
+                               pci_dev_put(pch);
+                               break;
                        }
-                       BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
                }
-               pci_dev_put(pch);
+               pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
+               pci_dev_put(curr);
+       }
+       if (!pch) {
+               DRM_DEBUG_KMS("No PCH found?\n");
        }
 }
_______________________________________________