[PATCH] drm/mgag200: Don't set <rammapen> in MISC

[Thomas Zimmermann]

Hi

Am 18.06.20 um 03:40 schrieb Rong Chen:
> 
> 
> On 6/17/20 8:29 PM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 17.06.20 um 11:22 schrieb Rong Chen:
>>> On Wed, Jun 17, 2020 at 08:28:02AM +0200, Thomas Zimmermann wrote:
>>>> Hi Emil
>>>>
>>>> Am 16.06.20 um 17:14 schrieb Emil Velikov:
>>>>> Hi Thomas,
>>>>>
>>>>> On Tue, 16 Jun 2020 at 15:26, Thomas Zimmermann
>>>>> <tzimmermann at suse.de> wrote:
>>>>>> The original modesetting code set MISC to 0x2d, which is <hpgoddev>,
>>>>>> <clksel> and <ioaddsel>
>>>>>>
>>>>>> With the conversion to atomic modesetting, <rammapen> accidentally
>>>>>> got enabled as well. Revert this change and initialize MISC with a
>>>>>> constant value of <hgoddev> and <ioaddsel>. The <clksel> bits are set
>>>>>> in mga_crtc_set_plls(), sync flags are set in
>>>>>> mgag200_set_mode_regs().
>>>>>>
>>>>> Let's keep the remove (restoring original functionality) and rename
>>>>> (cosmetics) separate patches. The read has also disappeared, which
>>>>> should be safe although might be better on it's own.
>>>> I'm waiting for Rong Chen's performance results on this patch. Moving
>>>> the rename into a separate patch makes sense, but removing the read is
>>>> part of restoring the original behavior. I think it should be in this
>>>> patch. Maybe I can write a better commit message to highlight the
>>>> change.
>>>>
>>> Hi Thomas,
>>>
>>> I tested the patch based on previous patch series, it seems doesn't take
>>> effect, and commit 39fb72816c4ee brings a larger regression when
>>> comparing to commit caac4dda44f37:
>> Thanks for testing!
>>
>> I still don't understand these numbers, but are you sure that
>> e44e907dd8f93 is really the regression?
> 
> Yes, it's a regression too, according to the previous report
> https://www.spinics.net/lists/dri-devel/msg260034.html
> the bot bisected to commit e44e907dd8f93 which let the
> glmark2.800x600.score reduced by 64.9%,
> but the bot doesn't notice the improvement of commit caac4dda44f37, and
> commit 39fb72816c4ee
> causes another regression.

I sent out a patch for 39fb72816c4ee. It enables caching on the
framebuffer's pages.

Best regards
Thomas

> 
> Best Regards,
> Rong Chen
> 
>>
>> It would make sense that 39fb72816c4ee has an impact on performance, as
>> it changes memory management and the way the driver updates the display.
>> I'll take a closer look at that patch and maybe send out an update.
>>
>> Best regards
>> Thomas
>>
>>> 4606edf870927 drm/mgag200: Don't set <rammapen> in
>>> MISC                                              3 3 3
>>> 39fb72816c4ee drm/mgag200: Replace VRAM helpers with SHMEM
>>> helpers                                   3 22
>>> caac4dda44f37 drm/mgag200: Convert to simple KMS
>>> helper                                              238 233 240 234 239
>>> 4b11c90431108 drm/mgag200: Use simple-display data
>>> structures                                        35 34 34 34
>>> db22c903c9322 drm/mgag200: Remove out-commented suspend/resume
>>> helpers                               34 34
>>> 87d4880d2aeb5 drm/mgag200: Move register initialization into separate
>>> function                       35
>>> 0c51726f95396 drm/mgag200: Move hiprilvl setting into separate
>>> functions                             35
>>> 8078e8b182e73 drm/mgag200: Set primary plane's format in separate
>>> helper function                    35
>>> 19f7b409d95b7 drm/mgag200: Set pitch in a separate helper
>>> function                                   35 34
>>> 9b9a363828c60 drm/mgag200: Update mode registers after plane
>>> registers                               34
>>> e44e907dd8f93 drm/mgag200: Split MISC register update into PLL
>>> selection, SYNC and I/O               34 34 34 34
>>> bef2303526adb drm/mgag200: Move mode-setting code into separate
>>> helper function                      97 97
>>> 5cd8460e81e8f drm/mgag200: Clean up
>>> mga_crtc_do_set_base()                                           96
>>> 0671ca8c559ba drm/mgag200: Clean up mga_set_start_address()
>>> e82c8969a2474 drm/mgag200: Remove HW
>>> cursor                                                          96
>>>
>>> Best Regards,
>>> Rong Chen
>>>
> 

-- 
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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200618/9b9c50f5/attachment-0001.sig>