| Message ID | 20200125195506.GA16638@brightrain.aerifal.cx (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | matroxfb: add Matrox MGA-G200eW board support | expand |
On Sat, Jan 25, 2020 at 02:55:06PM -0500, Rich Felker wrote: > Signed-off-by: Rich Felker <dalias@libc.org> > -- I know I don't accept patches without any changelog text, don't know about other subsystem maintainers... greg k-h
Hi Am 25.01.20 um 20:55 schrieb Rich Felker: > Signed-off-by: Rich Felker <dalias@libc.org> > -- > I've had this lying around a while and figure I should send it > upsteam; it's needed to support the onboard video on my Spectre-free > Atom S1260 server board. This HW is supported by mgag200, which is maintained. Can't you use that? Best regards Thomas > > --- > drivers/video/fbdev/matrox/matroxfb_base.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c > index 1a555f70923a..ff344313860c 100644 > --- a/drivers/video/fbdev/matrox/matroxfb_base.c > +++ b/drivers/video/fbdev/matrox/matroxfb_base.c > @@ -1376,6 +1376,12 @@ static struct video_board vbG200 = { > .accelID = FB_ACCEL_MATROX_MGAG200, > .lowlevel = &matrox_G100 > }; > +static struct video_board vbG200eW = { > + .maxvram = 0x800000, > + .maxdisplayable = 0x800000, > + .accelID = FB_ACCEL_MATROX_MGAG200, > + .lowlevel = &matrox_G100 > +}; > /* from doc it looks like that accelerator can draw only to low 16MB :-( Direct accesses & displaying are OK for > whole 32MB */ > static struct video_board vbG400 = { > @@ -1494,6 +1500,13 @@ static struct board { > MGA_G200, > &vbG200, > "MGA-G200 (PCI)"}, > + {PCI_VENDOR_ID_MATROX, 0x0532, 0xFF, > + 0, 0, > + DEVF_G200, > + 250000, > + MGA_G200, > + &vbG200eW, > + "MGA-G200eW (PCI)"}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, 0xFF, > PCI_SS_VENDOR_ID_MATROX, PCI_SS_ID_MATROX_GENERIC, > DEVF_G200, > @@ -2136,6 +2149,8 @@ static const struct pci_device_id matroxfb_devices[] = { > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_PCI, > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > + {PCI_VENDOR_ID_MATROX, 0x0532, > + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G400, >
Hi Greg, On Sun, Jan 26, 2020 at 8:44 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Sat, Jan 25, 2020 at 02:55:06PM -0500, Rich Felker wrote: > > Signed-off-by: Rich Felker <dalias@libc.org> > > -- > > I know I don't accept patches without any changelog text, don't know > about other subsystem maintainers... FTR, I do, iff the one-line summary says everything that needs to be said. What's the point in writing a full paragraph like: Currently the foo driver does not support the bar device. As users may want to use the bar device, it makes perfect sense to add support for the bar device to the foo driver. Hence add support for the bar device to the foo driver. if this doesn't add any value on top of the one-line summary? Gr{oetje,eeting}s, Geert
On Mon, Jan 27, 2020 at 11:40:24AM +0100, Geert Uytterhoeven wrote: > Hi Greg, > > On Sun, Jan 26, 2020 at 8:44 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Sat, Jan 25, 2020 at 02:55:06PM -0500, Rich Felker wrote: > > > Signed-off-by: Rich Felker <dalias@libc.org> > > > -- > > > > I know I don't accept patches without any changelog text, don't know > > about other subsystem maintainers... > > FTR, I do, iff the one-line summary says everything that needs to be said. > > What's the point in writing a full paragraph like: > > Currently the foo driver does not support the bar device. > As users may want to use the bar device, it makes perfect sense > to add support for the bar device to the foo driver. > Hence add support for the bar device to the foo driver. > > if this doesn't add any value on top of the one-line summary? At least it allows one to reply to *something*. If there's just a subject line you have zero quoted context for the reply apart from the patch itself. So rather confusing if you want to comment on the overall thing rather than on any specific changes in the diff. The other bad commit message style I dislike is the: "Subject: Do something... ...because whatever." As if the subject was a part of the first sentence of the commit message. Often makes replies even more confusing since now you have just a part of the sentece quoted. I do understand why new people make this mistake though; There should probably be a more explicit indication that the first line is the subject when editing the commit message.
On Mon, Jan 27, 2020 at 08:36:07AM +0100, Thomas Zimmermann wrote: > Hi > > Am 25.01.20 um 20:55 schrieb Rich Felker: > > Signed-off-by: Rich Felker <dalias@libc.org> > > -- > > I've had this lying around a while and figure I should send it > > upsteam; it's needed to support the onboard video on my Spectre-free > > Atom S1260 server board. > > This HW is supported by mgag200, which is maintained. Can't you use that? Perhaps; I wasn't aware it existed. I'll give it a try. It still might be nice to apply my patch though since the matroxfb driver works with it and only fails to support it because of not knowing the device id. Rich
Hi Am 28.01.20 um 19:58 schrieb Rich Felker: > On Mon, Jan 27, 2020 at 08:36:07AM +0100, Thomas Zimmermann wrote: >> Hi >> >> Am 25.01.20 um 20:55 schrieb Rich Felker: >>> Signed-off-by: Rich Felker <dalias@libc.org> >>> -- >>> I've had this lying around a while and figure I should send it >>> upsteam; it's needed to support the onboard video on my Spectre-free >>> Atom S1260 server board. >> >> This HW is supported by mgag200, which is maintained. Can't you use that? > > Perhaps; I wasn't aware it existed. I'll give it a try. It still might > be nice to apply my patch though since the matroxfb driver works with > it and only fails to support it because of not knowing the device id. Well, I have no say about applying your patch. You can ping me however, if mgag200 doesn't work for you. Best regards Thomas > > Rich >
On 1/25/20 8:55 PM, Rich Felker wrote: > Signed-off-by: Rich Felker <dalias@libc.org> Patch queued for v5.7, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > -- > I've had this lying around a while and figure I should send it > upsteam; it's needed to support the onboard video on my Spectre-free > Atom S1260 server board. > > --- > drivers/video/fbdev/matrox/matroxfb_base.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c > index 1a555f70923a..ff344313860c 100644 > --- a/drivers/video/fbdev/matrox/matroxfb_base.c > +++ b/drivers/video/fbdev/matrox/matroxfb_base.c > @@ -1376,6 +1376,12 @@ static struct video_board vbG200 = { > .accelID = FB_ACCEL_MATROX_MGAG200, > .lowlevel = &matrox_G100 > }; > +static struct video_board vbG200eW = { > + .maxvram = 0x800000, > + .maxdisplayable = 0x800000, > + .accelID = FB_ACCEL_MATROX_MGAG200, > + .lowlevel = &matrox_G100 > +}; > /* from doc it looks like that accelerator can draw only to low 16MB :-( Direct accesses & displaying are OK for > whole 32MB */ > static struct video_board vbG400 = { > @@ -1494,6 +1500,13 @@ static struct board { > MGA_G200, > &vbG200, > "MGA-G200 (PCI)"}, > + {PCI_VENDOR_ID_MATROX, 0x0532, 0xFF, > + 0, 0, > + DEVF_G200, > + 250000, > + MGA_G200, > + &vbG200eW, > + "MGA-G200eW (PCI)"}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, 0xFF, > PCI_SS_VENDOR_ID_MATROX, PCI_SS_ID_MATROX_GENERIC, > DEVF_G200, > @@ -2136,6 +2149,8 @@ static const struct pci_device_id matroxfb_devices[] = { > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_PCI, > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > + {PCI_VENDOR_ID_MATROX, 0x0532, > + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, > PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G400, >
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c index 1a555f70923a..ff344313860c 100644 --- a/drivers/video/fbdev/matrox/matroxfb_base.c +++ b/drivers/video/fbdev/matrox/matroxfb_base.c @@ -1376,6 +1376,12 @@ static struct video_board vbG200 = { .accelID = FB_ACCEL_MATROX_MGAG200, .lowlevel = &matrox_G100 }; +static struct video_board vbG200eW = { + .maxvram = 0x800000, + .maxdisplayable = 0x800000, + .accelID = FB_ACCEL_MATROX_MGAG200, + .lowlevel = &matrox_G100 +}; /* from doc it looks like that accelerator can draw only to low 16MB :-( Direct accesses & displaying are OK for whole 32MB */ static struct video_board vbG400 = { @@ -1494,6 +1500,13 @@ static struct board { MGA_G200, &vbG200, "MGA-G200 (PCI)"}, + {PCI_VENDOR_ID_MATROX, 0x0532, 0xFF, + 0, 0, + DEVF_G200, + 250000, + MGA_G200, + &vbG200eW, + "MGA-G200eW (PCI)"}, {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, 0xFF, PCI_SS_VENDOR_ID_MATROX, PCI_SS_ID_MATROX_GENERIC, DEVF_G200, @@ -2136,6 +2149,8 @@ static const struct pci_device_id matroxfb_devices[] = { PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_PCI, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, + {PCI_VENDOR_ID_MATROX, 0x0532, + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G200_AGP, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, {PCI_VENDOR_ID_MATROX, PCI_DEVICE_ID_MATROX_G400,
Signed-off-by: Rich Felker <dalias@libc.org> -- I've had this lying around a while and figure I should send it upsteam; it's needed to support the onboard video on my Spectre-free Atom S1260 server board. --- drivers/video/fbdev/matrox/matroxfb_base.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)