Hi Thomas.
@@ -1161,9 +1174,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, }
WREG_ECRT(0, ext_vga[0]);
- /* Enable mga pixel clock */
- misc = 0x2d;
- misc = RREG8(MGA_MISC_IN);
- misc |= MGAREG_MISC_IOADSEL |
MGAREG_MISC_RAMMAPEN |MGAREG_MISC_HIGH_PG_SEL;I am left puzzeled why there is several writes to MGA_MISC_OUT. The driver needs to read back and then write again.
Would it be simpler to have one function that only took care of misc register update?
I'm not quite sure what you mean. MISC contains all kinds of unrelated state. In the final atomic code, different state is set in different places. It's simple to have a function read-modify-write the content of MISC for the bits that it cares about. With multiple functions, that leads to some duplicated reads and write, but the code as a whole is simple.
OK, when I looked at the code I initially got the impression that it was a bit random. But then I also switched between patch and code etc. The explanation above makes sense.
Sam
Best regards Thomas
mga_crtc_do_set_base(mdev, fb, old_fb); diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index c096a9d6bcbc1..89e12c55153cf 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -16,10 +16,11 @@
MGA1064SG Mystique register file*/
#ifndef _MGA_REG_H_ #define _MGA_REG_H_
+#include <linux/bits.h>
#define MGAREG_DWGCTL 0x1c00 #define MGAREG_MACCESS 0x1c04 /* the following is a mystique only register */ @@ -227,6 +228,8 @@ #define MGAREG_MISC_CLK_SEL_MGA_MSK (0x3 << 2) #define MGAREG_MISC_VIDEO_DIS (0x1 << 4) #define MGAREG_MISC_HIGH_PG_SEL (0x1 << 5) +#define MGAREG_MISC_HSYNCPOL BIT(6) +#define MGAREG_MISC_VSYNCPOL BIT(7)
I like BIT(), but mixing (1 << N) and BIT() does not look nice. Oh, and do not get me started on GENMASK() :-)
Sam
/* MMIO VGA registers */
#define MGAREG_SEQ_INDEX 0x1fc4
2.26.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 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