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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 4895 bytes --]

Hi

Am 06.12.19 um 07:50 schrieb David Airlie:
> On Fri, Dec 6, 2019 at 4:14 PM Thomas Zimmermann <tzimmermann@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@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@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@suse.de>
>>>>>>>> Reported-by: John Donnelly <john.p.donnelly@oracle.com>
>>>>>>>> Link: https://gitlab.freedesktop.org/drm/misc/issues/7
>>>>>>>
>>>>>>> Tested-by: John Donnelly <john.p.donnelly@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@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@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.

OK, thanks. I'll prepare a patch to flag all 0x0522 machines.

Best regards
Thomas

> 
> Dave.
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel