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

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

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