Hi Thomas. On Tue, May 12, 2020 at 10:42:45AM +0200, Thomas Zimmermann wrote: > All register names and fields are now named according to the > MGA programming manuals. The function doesn't need the CRTC, so > callers pass in the device structure directly. The logging now > uses device-specific macros. > > v2: > * use to_mga_device() > * use MiB instead of MB > * replace empty while loop with do-while, fixes checkpatch warning > * replace uint{8,32}_t with u{8,32} > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > Tested-by: John Donnelly <John.p.donnelly@xxxxxxxxxx> With the comment below addressed: Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx> > --- > drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++ > drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++----------- > 2 files changed, 53 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h > index bc372c2ec79e9..1963876ef3b8c 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -61,6 +61,11 @@ > WREG8(MGAREG_CRTC_DATA, v); \ > } while (0) \ > > +#define RREG_ECRT(reg, v) \ > + do { \ > + WREG8(MGAREG_CRTCEXT_INDEX, reg); \ > + v = RREG8(MGAREG_CRTCEXT_DATA); \ > + } while (0) \ > > #define WREG_ECRT(reg, v) \ > do { \ > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c > index c68ed8b6faf9b..80a3a805c0c4e 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c > @@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc) > } > > /* > - This is how the framebuffer base address is stored in g200 cards: > - * Assume @offset is the gpu_addr variable of the framebuffer object > - * Then addr is the number of _pixels_ (not bytes) from the start of > - VRAM to the first pixel we want to display. (divided by 2 for 32bit > - framebuffers) > - * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers > - addr<20> -> CRTCEXT0<6> > - addr<19-16> -> CRTCEXT0<3-0> > - addr<15-8> -> CRTCC<7-0> > - addr<7-0> -> CRTCD<7-0> > - CRTCEXT0 has to be programmed last to trigger an update and make the > - new addr variable take effect. > + * This is how the framebuffer base address is stored in g200 cards: > + * * Assume @offset is the gpu_addr variable of the framebuffer object > + * * Then addr is the number of _pixels_ (not bytes) from the start of > + * VRAM to the first pixel we want to display. (divided by 2 for 32bit > + * framebuffers) > + * * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers > + * addr<20> -> CRTCEXT0<6> > + * addr<19-16> -> CRTCEXT0<3-0> > + * addr<15-8> -> CRTCC<7-0> > + * addr<7-0> -> CRTCD<7-0> > + * > + * CRTCEXT0 has to be programmed last to trigger an update and make the > + * new addr variable take effect. > */ > -static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset) > +static void mgag200_set_startadd(struct mga_device *mdev, > + unsigned long offset) > { > - struct mga_device *mdev = to_mga_device(crtc->dev); > - u32 addr; > - int count; > - u8 crtcext0; > + struct drm_device *dev = mdev->dev; > + u32 startadd; > + u8 crtcc, crtcd, crtcext0; > > - while (RREG8(0x1fda) & 0x08); > - while (!(RREG8(0x1fda) & 0x08)); > + startadd = offset / 8; > > - count = RREG8(MGAREG_VCOUNT) + 2; > - while (RREG8(MGAREG_VCOUNT) < count); > - > - WREG8(MGAREG_CRTCEXT_INDEX, 0); > - crtcext0 = RREG8(MGAREG_CRTCEXT_DATA); > - crtcext0 &= 0xB0; > - addr = offset / 8; > - /* Can't store addresses any higher than that... > - but we also don't have more than 16MB of memory, so it should be fine. */ > - WARN_ON(addr > 0x1fffff); > - crtcext0 |= (!!(addr & (1<<20)))<<6; > - WREG_CRT(0x0d, (u8)(addr & 0xff)); > - WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff); > - WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0); > + /* > + * Can't store addresses any higher than that, but we also > + * don't have more than 16 MiB of memory, so it should be fine. > + */ > + drm_WARN_ON(dev, startadd > 0x1fffff); > + > + RREG_ECRT(0x00, crtcext0); > + > + crtcc = (startadd >> 8) & 0xff; > + crtcd = startadd & 0xff; > + crtcext0 &= 0xb0; > + crtcext0 |= ((startadd >> 14) & BIT(6)) | > + ((startadd >> 16) & 0x0f); > + > + WREG_CRT(0x0c, crtcc); > + WREG_CRT(0x0d, crtcd); > + WREG_ECRT(0x00, crtcext0); > } > > static int mga_crtc_do_set_base(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > int x, int y, int atomic) > { > + struct mga_device *mdev = to_mga_device(crtc->dev); > struct drm_gem_vram_object *gbo; > int ret; > s64 gpu_addr; > @@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, > goto err_drm_gem_vram_unpin; > } > > - mga_set_start_address(crtc, (u32)gpu_addr); > + mgag200_set_startadd(mdev, (unsigned long)gpu_addr); > > return 0; > > @@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, > static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, > struct drm_framebuffer *old_fb) > { > + struct drm_device *dev = crtc->dev; > + struct mga_device *mdev = dev->dev_private; > + unsigned int count; > + > + do { } while (RREG8(0x1fda) & 0x08); > + do { } while (!(RREG8(0x1fda) & 0x08)); > + > + count = RREG8(MGAREG_VCOUNT) + 2; > + do { } while (RREG8(MGAREG_VCOUNT) < count); > + > return mga_crtc_do_set_base(crtc, old_fb, x, y, 0); > } The changelog still does not explain why this code is moved here. > > -- > 2.26.2 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel