All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: john.p.donnelly@oracle.com, dri-devel@lists.freedesktop.org,
	kraxel@redhat.com, airlied@redhat.com
Subject: Re: [PATCH 07/17] drm/mgag200: Move mode-setting code into separate helper function
Date: Wed, 29 Apr 2020 20:24:33 +0200	[thread overview]
Message-ID: <20200429182433.GG31071@ravnborg.org> (raw)
In-Reply-To: <20200429143238.10115-8-tzimmermann@suse.de>

On Wed, Apr 29, 2020 at 04:32:28PM +0200, Thomas Zimmermann wrote:
> The mode-setting code is now located in mgag200_set_mode_regs(), sans
> a few flags that will be moved in a later patch for clarity.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Looked like trivial code movements, did not look carefully.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 140 ++++++++++++++-----------
>  1 file changed, 78 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 12df809d64f7c..749ba6e420ac7 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -911,6 +911,79 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	return mga_crtc_do_set_base(mdev, fb, old_fb);
>  }
>  
> +static void mgag200_set_mode_regs(struct mga_device *mdev,
> +				  const struct drm_display_mode *mode)
> +{
> +	unsigned int hdisplay, hsyncstart, hsyncend, htotal;
> +	unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
> +	uint8_t misc = 0;
> +	uint8_t crtcext1, crtcext2, crtcext5;
> +
> +	hdisplay = mode->hdisplay / 8 - 1;
> +	hsyncstart = mode->hsync_start / 8 - 1;
> +	hsyncend = mode->hsync_end / 8 - 1;
> +	htotal = mode->htotal / 8 - 1;
> +
> +	/* Work around hardware quirk */
> +	if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
> +		htotal++;
> +
> +	vdisplay = mode->vdisplay - 1;
> +	vsyncstart = mode->vsync_start - 1;
> +	vsyncend = mode->vsync_end - 1;
> +	vtotal = mode->vtotal - 2;
> +
> +	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> +		misc |= 0x40;
> +	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> +		misc |= 0x80;
> +
> +	crtcext1 = (((htotal - 4) & 0x100) >> 8) |
> +		   ((hdisplay & 0x100) >> 7) |
> +		   ((hsyncstart & 0x100) >> 6) |
> +		    (htotal & 0x40);
> +	if (mdev->type == G200_WB || mdev->type == G200_EW3)
> +		crtcext1 |= BIT(7) | /* vrsten */
> +			    BIT(3); /* hrsten */
> +
> +	crtcext2 = ((vtotal & 0xc00) >> 10) |
> +		   ((vdisplay & 0x400) >> 8) |
> +		   ((vdisplay & 0xc00) >> 7) |
> +		   ((vsyncstart & 0xc00) >> 5) |
> +		   ((vdisplay & 0x400) >> 3);
> +	crtcext5 = 0x00;
> +
> +	WREG_CRT(0, htotal - 4);
> +	WREG_CRT(1, hdisplay);
> +	WREG_CRT(2, hdisplay);
> +	WREG_CRT(3, (htotal & 0x1F) | 0x80);
> +	WREG_CRT(4, hsyncstart);
> +	WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
> +	WREG_CRT(6, vtotal & 0xFF);
> +	WREG_CRT(7, ((vtotal & 0x100) >> 8) |
> +		 ((vdisplay & 0x100) >> 7) |
> +		 ((vsyncstart & 0x100) >> 6) |
> +		 ((vdisplay & 0x100) >> 5) |
> +		 ((vdisplay & 0x100) >> 4) | /* linecomp */
> +		 ((vtotal & 0x200) >> 4) |
> +		 ((vdisplay & 0x200) >> 3) |
> +		 ((vsyncstart & 0x200) >> 2));
> +	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
> +		 ((vdisplay & 0x200) >> 3));
> +	WREG_CRT(16, vsyncstart & 0xFF);
> +	WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
> +	WREG_CRT(18, vdisplay & 0xFF);
> +	WREG_CRT(20, 0);
> +	WREG_CRT(21, vdisplay & 0xFF);
> +	WREG_CRT(22, (vtotal + 1) & 0xFF);
> +	WREG_CRT(23, 0xc3);
> +	WREG_CRT(24, vdisplay & 0xFF);
> +
> +	WREG_ECRT(0x01, crtcext1);
> +	WREG_ECRT(0x02, crtcext2);
> +	WREG_ECRT(0x05, crtcext5);
> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  				struct drm_display_mode *mode,
>  				struct drm_display_mode *adjusted_mode,
> @@ -919,8 +992,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	struct mga_device *mdev = dev->dev_private;
>  	const struct drm_framebuffer *fb = crtc->primary->fb;
> -	int hdisplay, hsyncstart, hsyncend, htotal;
> -	int vdisplay, vsyncstart, vsyncend, vtotal;
>  	int pitch;
>  	int option = 0, option2 = 0;
>  	int i;
> @@ -999,12 +1070,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  		break;
>  	}
>  
> -	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> -		misc |= 0x40;
> -	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> -		misc |= 0x80;
> -
> -
>  	for (i = 0; i < sizeof(dacvalue); i++) {
>  		if ((i <= 0x17) ||
>  		    (i == 0x1b) ||
> @@ -1044,20 +1109,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	else
>  		pitch = pitch >> (4 - bppshift);
>  
> -	hdisplay = mode->hdisplay / 8 - 1;
> -	hsyncstart = mode->hsync_start / 8 - 1;
> -	hsyncend = mode->hsync_end / 8 - 1;
> -	htotal = mode->htotal / 8 - 1;
> -
> -	/* Work around hardware quirk */
> -	if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
> -		htotal++;
> -
> -	vdisplay = mode->vdisplay - 1;
> -	vsyncstart = mode->vsync_start - 1;
> -	vsyncend = mode->vsync_end - 1;
> -	vtotal = mode->vtotal - 2;
> -
>  	WREG_GFX(0, 0);
>  	WREG_GFX(1, 0);
>  	WREG_GFX(2, 0);
> @@ -1068,61 +1119,26 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	WREG_GFX(7, 0xf);
>  	WREG_GFX(8, 0xf);
>  
> -	WREG_CRT(0, htotal - 4);
> -	WREG_CRT(1, hdisplay);
> -	WREG_CRT(2, hdisplay);
> -	WREG_CRT(3, (htotal & 0x1F) | 0x80);
> -	WREG_CRT(4, hsyncstart);
> -	WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
> -	WREG_CRT(6, vtotal & 0xFF);
> -	WREG_CRT(7, ((vtotal & 0x100) >> 8) |
> -		 ((vdisplay & 0x100) >> 7) |
> -		 ((vsyncstart & 0x100) >> 6) |
> -		 ((vdisplay & 0x100) >> 5) |
> -		 ((vdisplay & 0x100) >> 4) | /* linecomp */
> -		 ((vtotal & 0x200) >> 4)|
> -		 ((vdisplay & 0x200) >> 3) |
> -		 ((vsyncstart & 0x200) >> 2));
> -	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
> -		 ((vdisplay & 0x200) >> 3));
>  	WREG_CRT(10, 0);
>  	WREG_CRT(11, 0);
>  	WREG_CRT(12, 0);
>  	WREG_CRT(13, 0);
>  	WREG_CRT(14, 0);
>  	WREG_CRT(15, 0);
> -	WREG_CRT(16, vsyncstart & 0xFF);
> -	WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
> -	WREG_CRT(18, vdisplay & 0xFF);
>  	WREG_CRT(19, pitch & 0xFF);
> -	WREG_CRT(20, 0);
> -	WREG_CRT(21, vdisplay & 0xFF);
> -	WREG_CRT(22, (vtotal + 1) & 0xFF);
> -	WREG_CRT(23, 0xc3);
> -	WREG_CRT(24, vdisplay & 0xFF);
> +
> +	mgag200_set_mode_regs(mdev, mode);
>  
>  	ext_vga[0] = 0;
> -	ext_vga[5] = 0;
>  
>  	/* TODO interlace */
>  
>  	ext_vga[0] |= (pitch & 0x300) >> 4;
> -	ext_vga[1] = (((htotal - 4) & 0x100) >> 8) |
> -		((hdisplay & 0x100) >> 7) |
> -		((hsyncstart & 0x100) >> 6) |
> -		(htotal & 0x40);
> -	ext_vga[2] = ((vtotal & 0xc00) >> 10) |
> -		((vdisplay & 0x400) >> 8) |
> -		((vdisplay & 0xc00) >> 7) |
> -		((vsyncstart & 0xc00) >> 5) |
> -		((vdisplay & 0x400) >> 3);
>  	if (fb->format->cpp[0] * 8 == 24)
>  		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
>  	else
>  		ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>  	ext_vga[4] = 0;
> -	if (mdev->type == G200_WB || mdev->type == G200_EW3)
> -		ext_vga[1] |= 0x88;
>  
>  	/* Set pixel clocks */
>  	misc = 0x2d;
> @@ -1130,9 +1146,9 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	mga_crtc_set_plls(mdev, mode->clock);
>  
> -	for (i = 0; i < 6; i++) {
> -		WREG_ECRT(i, ext_vga[i]);
> -	}
> +	WREG_ECRT(0, ext_vga[0]);
> +	WREG_ECRT(3, ext_vga[3]);
> +	WREG_ECRT(4, ext_vga[4]);
>  
>  	if (mdev->type == G200_ER)
>  		WREG_ECRT(0x24, 0x5);
> -- 
> 2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-04-29 18:24 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 14:32 [PATCH 00/17] drm/mgag200: Convert to atomic modesetting Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 01/17] drm/mgag200: Remove HW cursor Thomas Zimmermann
2020-04-29 17:51   ` Sam Ravnborg
2020-04-30  7:03     ` Gerd Hoffmann
2020-04-30  8:10     ` Thomas Zimmermann
2020-04-30  9:19       ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 02/17] drm/mgag200: Remove unused fields from struct mga_device Thomas Zimmermann
2020-04-29 17:49   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 03/17] drm/mgag200: Embed connector instance in " Thomas Zimmermann
2020-04-29 15:24   ` Ruhl, Michael J
2020-04-29 17:49   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 04/17] drm/mgag200: Use managed mode-config initialization Thomas Zimmermann
2020-04-29 17:55   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 05/17] drm/mgag200: Clean up mga_set_start_address() Thomas Zimmermann
2020-04-29 18:20   ` Sam Ravnborg
2020-04-30  8:23     ` Thomas Zimmermann
2020-05-11 12:41       ` Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 06/17] drm/mgag200: Clean up mga_crtc_do_set_base() Thomas Zimmermann
2020-04-29 18:23   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 07/17] drm/mgag200: Move mode-setting code into separate helper function Thomas Zimmermann
2020-04-29 18:24   ` Sam Ravnborg [this message]
2020-04-30  8:27     ` Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 08/17] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O Thomas Zimmermann
2020-05-03 15:34   ` Sam Ravnborg
2020-05-04 13:03     ` Thomas Zimmermann
2020-05-04 14:25       ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 09/17] drm/mgag200: Update mode registers after plane registers Thomas Zimmermann
2020-05-03 15:34   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 10/17] drm/mgag200: Set pitch in a separate helper function Thomas Zimmermann
2020-05-03 15:42   ` Sam Ravnborg
2020-05-04 13:10     ` Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 11/17] drm/mgag200: Set primary plane's format in " Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 12/17] drm/mgag200: Move TAGFIFO reset into separate function Thomas Zimmermann
2020-05-03 16:25   ` Sam Ravnborg
2020-05-04 13:11     ` Thomas Zimmermann
2020-05-04 14:29       ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 13/17] drm/mgag200: Move hiprilvl setting into separate functions Thomas Zimmermann
2020-05-03 17:23   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 14/17] drm/mgag200: Move register initialization into separate function Thomas Zimmermann
2020-05-03 17:25   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 15/17] drm/mgag200: Remove waiting from DPMS code Thomas Zimmermann
2020-05-04 12:10   ` Daniel Vetter
2020-05-04 12:40     ` Thomas Zimmermann
2020-04-29 14:32 ` [PATCH 16/17] drm/mgag200: Convert to simple KMS helper Thomas Zimmermann
2020-05-03 17:36   ` Sam Ravnborg
2020-04-29 14:32 ` [PATCH 17/17] drm/mgag200: Replace VRAM helpers with SHMEM helpers Thomas Zimmermann
2020-05-04 12:29   ` Emil Velikov
2020-05-04 12:45     ` Thomas Zimmermann
2020-04-30  0:11 ` [PATCH 00/17] drm/mgag200: Convert to atomic modesetting John Donnelly
2020-04-30  8:29   ` Thomas Zimmermann
2020-04-30 12:09     ` John Donnelly
2020-05-04 13:39   ` Thomas Zimmermann
2020-05-04 20:39     ` John Donnelly
2020-05-05 12:20     ` John Donnelly
2020-05-06  7:29       ` Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429182433.GG31071@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=john.p.donnelly@oracle.com \
    --cc=kraxel@redhat.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.