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

From: Rong Chen <rong.a.chen@intel.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: "John Donnelly" <john.p.donnelly@oracle.com>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Emil Velikov" <emil.l.velikov@gmail.com>,
	"ML dri-devel" <dri-devel@lists.freedesktop.org>,
	"Andrzej Pietrasiewicz" <andrzej.p@collabora.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"José Roberto de Souza" <jose.souza@intel.com>,
	"Dave Airlie" <airlied@redhat.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Emil Velikov" <emil.velikov@collabora.com>
Subject: Re: [PATCH] drm/mgag200: Don't set <rammapen> in MISC
Date: Wed, 17 Jun 2020 17:22:52 +0800	[thread overview]
Message-ID: <20200617092252.GA5279@shao2-debian> (raw)
In-Reply-To: <ea09692a-5a8f-d315-3bce-d416e8a01c7a@suse.de>

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

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel