| Message ID | 20220426164108.1051295-4-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mgag200: Improve damage handling | expand |
Hi Am 26.04.22 um 18:41 schrieb Jocelyn Falempe: > The flag MGAG200_FLAG_HW_BUG_NO_STARTADD is no more used, because > the framebuffer is now always at offset 0. Oh, well. I remember that thing. It took us a long time to find and fix this problem. Back then, mgag200 still used VRAM helpers, which do page flipping in video memory. Displays remained dark on some systems without any clear cause. We added the flag to work around the broken HW. I left the flag in for reference. Instead of removing it, I think we should add a drm_WARN_ON_ONCE() in mgag200_set_start_add() if the flag is set and offset is non-zero. Best regards Thomas > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- > drivers/gpu/drm/mgag200/mgag200_drv.h | 3 --- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c > index 217844d71ab5..8659e1ca8009 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -306,8 +306,7 @@ mgag200_device_create(struct pci_dev *pdev, enum mga_type type, unsigned long fl > static const struct pci_device_id mgag200_pciidlist[] = { > { PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_PCI }, > { PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_AGP }, > - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, > - G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, > + { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A }, > { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, > { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV }, > { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_WB }, > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h > index 4368112023f7..c7b6dc771ab3 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -201,9 +201,6 @@ enum mga_type { > G200_EW3, > }; > > -/* HW does not handle 'startadd' field correct. */ > -#define MGAG200_FLAG_HW_BUG_NO_STARTADD (1ul << 8) > - > #define MGAG200_TYPE_MASK (0x000000ff) > #define MGAG200_FLAG_MASK (0x00ffff00) >
On 04/05/2022 12:12, Thomas Zimmermann wrote: > Hi > > Am 26.04.22 um 18:41 schrieb Jocelyn Falempe: >> The flag MGAG200_FLAG_HW_BUG_NO_STARTADD is no more used, because >> the framebuffer is now always at offset 0. > > Oh, well. I remember that thing. It took us a long time to find and fix > this problem. Back then, mgag200 still used VRAM helpers, which do page > flipping in video memory. Displays remained dark on some systems without > any clear cause. We added the flag to work around the broken HW. > > I left the flag in for reference. Instead of removing it, I think we > should add a drm_WARN_ON_ONCE() in mgag200_set_start_add() if the flag > is set and offset is non-zero. sure, I can do that in v2. > > Best regards > Thomas > >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- >> drivers/gpu/drm/mgag200/mgag200_drv.h | 3 --- >> 2 files changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c >> b/drivers/gpu/drm/mgag200/mgag200_drv.c >> index 217844d71ab5..8659e1ca8009 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c >> @@ -306,8 +306,7 @@ mgag200_device_create(struct pci_dev *pdev, enum >> mga_type type, unsigned long fl >> static const struct pci_device_id mgag200_pciidlist[] = { >> { PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_PCI }, >> { PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_AGP }, >> - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> - G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, >> + { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_SE_A }, >> { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_SE_B }, >> { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_EV }, >> { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> G200_WB }, >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h >> b/drivers/gpu/drm/mgag200/mgag200_drv.h >> index 4368112023f7..c7b6dc771ab3 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h >> @@ -201,9 +201,6 @@ enum mga_type { >> G200_EW3, >> }; >> -/* HW does not handle 'startadd' field correct. */ >> -#define MGAG200_FLAG_HW_BUG_NO_STARTADD (1ul << 8) >> - >> #define MGAG200_TYPE_MASK (0x000000ff) >> #define MGAG200_FLAG_MASK (0x00ffff00) >
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 217844d71ab5..8659e1ca8009 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -306,8 +306,7 @@ mgag200_device_create(struct pci_dev *pdev, enum mga_type type, unsigned long fl static const struct pci_device_id mgag200_pciidlist[] = { { PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_PCI }, { PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_AGP }, - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, - G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, + { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A }, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV }, { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_WB }, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 4368112023f7..c7b6dc771ab3 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -201,9 +201,6 @@ enum mga_type { G200_EW3, }; -/* HW does not handle 'startadd' field correct. */ -#define MGAG200_FLAG_HW_BUG_NO_STARTADD (1ul << 8) - #define MGAG200_TYPE_MASK (0x000000ff) #define MGAG200_FLAG_MASK (0x00ffff00)
The flag MGAG200_FLAG_HW_BUG_NO_STARTADD is no more used, because the framebuffer is now always at offset 0. Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- drivers/gpu/drm/mgag200/mgag200_drv.h | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-)