[PATCH 3/4] drm/mgag200: Add workaround for HW that does not support 'startadd'

[David Airlie]

On Fri, Dec 6, 2019 at 4:14 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
>
> Hi
>
> Am 04.12.19 um 10:36 schrieb Dave Airlie:
> > On Wed, 4 Dec 2019 at 17:30, Thomas Zimmermann <tzimmermann at suse.de> wrote:
> >>
> >> Hi John
> >>
> >> Am 03.12.19 um 18:55 schrieb John Donnelly:
> >>> Hi ,
> >>>
> >>> See below ,
> >>>
> >>>
> >>>> On Nov 26, 2019, at 3:50 AM, Thomas Zimmermann <tzimmermann at suse.de> wrote:
> >>>>
> >>>> Hi
> >>>>
> >>>> Am 26.11.19 um 10:37 schrieb Daniel Vetter:
> >>>>> On Tue, Nov 26, 2019 at 08:25:44AM +0100, Thomas Zimmermann wrote:
> >>>>>> There's at least one system that does not interpret the value of
> >>>>>> the device's 'startadd' field correctly, which leads to incorrectly
> >>>>>> displayed scanout buffers. Always placing the active scanout buffer
> >>>>>> at offset 0 works around the problem.
> >>>>>>
> >>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> >>>>>> Reported-by: John Donnelly <john.p.donnelly at oracle.com>
> >>>>>> Link: https://gitlab.freedesktop.org/drm/misc/issues/7
> >>>>>
> >>>>> Tested-by: John Donnelly <john.p.donnelly at oracle.com>
> >>>>>
> >>>>> (Not quite this patch, but pretty much the logic, so counts).
> >>>>>
> >>>>> Fixes: 81da87f63a1e ("drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin")
> >>>>> Cc: <stable at vger.kernel.org> # v5.3+
> >>>>>
> >>>>> Also you need the stable line on both prep patches too. For next time
> >>>>> around,
> >>>>>
> >>>>> $ dim fixes 81da87f63a1e
> >>>>>
> >>>>> will generate all the stuff you need, including a good set of suggested
> >>>>> Cc: you should have.
> >>>>>
> >>>>> On the first 3 patches, with all that stuff added:
> >>>>>
> >>>>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >>>>
> >>>> Thanks for the review.
> >>>>
> >>>> Sorry for leaving out some of the tags. I wanted to wait for feedback
> >>>> before adding tested-by, fixes, stable. I'll split off patch 4 from the
> >>>> series and get 1 to 3 merged ASAP.
> >>>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>>>
> >>>>> Please push these to drm-misc-next-fixes so they get backported as quickly
> >>>>> as possible.
> >>>>> -Daniel
> >>>>>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/mgag200/mgag200_drv.c | 36 ++++++++++++++++++++++++++-
> >>>>>> drivers/gpu/drm/mgag200/mgag200_drv.h |  3 +++
> >>>>>> 2 files changed, 38 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> >>>>>> index 397f8b0a9af8..d43951caeea0 100644
> >>>>>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> >>>>>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> >>>>>> @@ -30,6 +30,8 @@ module_param_named(modeset, mgag200_modeset, int, 0400);
> >>>>>> static struct drm_driver driver;
> >>>>>>
> >>>>>> static const struct pci_device_id pciidlist[] = {
> >>>>>> +  { PCI_VENDOR_ID_MATROX, 0x522, PCI_VENDOR_ID_SUN, 0x4852, 0, 0,
> >>>>>> +          G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD},
> >>>
> >>>
> >>>
> >>> I will have an additional list of vendor IDs (0x4852)  I will provide in a follow up patch shortly .  This fixes only 1 flavor  of server.
> >>
> >> Thank you for all the testing you do. We can add these ids as necessary.
> >>
> >> Before, I posted a patch at [1] that prints an internal unique id. The
> >> id of your original test machine was 0x2 IIRC.
> >>
> >> My guess is that the problem might be related to the chip's revision. If
> >> you have the option of booting your own kernels on all these machines,
> >> could you apply the patch and look for a pattern in these ids? Maybe
> >> only revision 0x2 is affected.
> >>
> >
> > I've got an old bug I never got around to that has a comment from Matrox
> >
> > "The issue is reproducible with G200e chip. The device ID is 0x0522."
> >
> > so it might be a broader problem than we think.
>
> Did they tell you a subvendor id? John reported that the internal
> revision id differs among affected machines. I'm thinking about flagging
> either Sun devices or all 0x0522 devices as broken.

Well it was from Matrox themselves, so they are the vendor ID, it
didn't sounds like subvendor mattered, though I expect the problem is
the BMC firmware anyways, not sure if we can even know what ths is.

Dave.