[Intel-gfx] [PATCH] Review i830_pad_drawable_width()
Matt Turner
mattst88 at gmail.com
Tue Mar 30 22:55:50 CEST 2010
On Tue, Mar 30, 2010 at 4:13 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> We appear to have a confusion of stride in terms of pixels, pitch in
> terms of bytes and the actual width of the surface.
> i830_pad_drawable_width() appears to be operating aligning *pixels* to a
> 64 pixel boundary and has never used the chars-per-pixel causing
> considerable confusion in its callers. Remove the parameter and ensure
> that the callers are expecting a value in pixels returned, multiplying
> by cpp where necessary to get the pitch.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> src/drmmode_display.c | 18 +++++++++---------
> src/i830.h | 11 +++++++++--
> src/i830_driver.c | 17 +++++------------
> 3 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 1348e08..d8b158e 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -476,7 +476,7 @@ drmmode_crtc_shadow_allocate(xf86CrtcPtr crtc, int width, int height)
> int size, ret;
> unsigned long rotate_pitch;
>
> - width = i830_pad_drawable_width(width, drmmode->cpp);
> + width = i830_pad_drawable_width(width);
> rotate_pitch = width * drmmode->cpp;
> size = rotate_pitch * height;
>
> @@ -523,8 +523,7 @@ drmmode_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height)
> }
> }
>
> - rotate_pitch =
> - i830_pad_drawable_width(width, drmmode->cpp) * drmmode->cpp;
> + rotate_pitch = i830_pad_drawable_width(width) * drmmode->cpp;
> rotate_pixmap = GetScratchPixmapHeader(scrn->pScreen,
> width, height,
> scrn->depth,
> @@ -1257,13 +1256,14 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
> Bool ret;
> ScreenPtr screen = screenInfo.screens[scrn->scrnIndex];
> uint32_t old_fb_id;
> - int i, pitch, old_width, old_height, old_pitch;
> + int i, w, pitch, old_width, old_height, old_pitch;
>
> if (scrn->virtualX == width && scrn->virtualY == height)
> return TRUE;
>
> - pitch = i830_pad_drawable_width(width, intel->cpp);
> - i830_tiled_width(intel, &pitch, intel->cpp);
> + w = i830_pad_drawable_width(width);
> + i830_tiled_width(intel, &w, intel->cpp);
> + pitch = w * intel->cpp;
> xf86DrvMsg(scrn->scrnIndex, X_INFO,
> "Allocate new frame buffer %dx%d stride %d\n",
> width, height, pitch);
> @@ -1276,13 +1276,13 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
>
> scrn->virtualX = width;
> scrn->virtualY = height;
> - scrn->displayWidth = pitch;
> + scrn->displayWidth = w;
> intel->front_buffer = i830_allocate_framebuffer(scrn);
> if (!intel->front_buffer)
> goto fail;
>
> ret = drmModeAddFB(drmmode->fd, width, height, scrn->depth,
> - scrn->bitsPerPixel, pitch * intel->cpp,
> + scrn->bitsPerPixel, pitch,
> intel->front_buffer->handle,
> &drmmode->fb_id);
> if (ret)
> @@ -1291,7 +1291,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
> i830_set_pixmap_bo(screen->GetScreenPixmap(screen), intel->front_buffer);
>
> screen->ModifyPixmapHeader(screen->GetScreenPixmap(screen),
> - width, height, -1, -1, pitch * intel->cpp, NULL);
> + width, height, -1, -1, pitch, NULL);
>
> for (i = 0; i < xf86_config->num_crtc; i++) {
> xf86CrtcPtr crtc = xf86_config->crtc[i];
> diff --git a/src/i830.h b/src/i830.h
> index 7593cde..43c5887 100644
> --- a/src/i830.h
> +++ b/src/i830.h
> @@ -429,8 +429,6 @@ void i830_init_bufmgr(ScrnInfoPtr scrn);
>
> Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp);
>
> -int i830_pad_drawable_width(int width, int cpp);
> -
> /* i830_memory.c */
> unsigned long i830_get_fence_size(intel_screen_private *intel, unsigned long size);
> unsigned long i830_get_fence_pitch(intel_screen_private *intel, unsigned long pitch,
> @@ -587,4 +585,13 @@ static inline Bool pixmap_is_scanout(PixmapPtr pixmap)
> return pixmap == screen->GetScreenPixmap(screen);
> }
>
> +/*
> + * Pad to accelerator requirement
> + */
> +static inline int i830_pad_drawable_width(int width)
> +{
> + return (width + 63) & ~63;
> +}
> +
> +
> #endif /* _I830_H_ */
> diff --git a/src/i830_driver.c b/src/i830_driver.c
> index 22e8472..148675e 100644
> --- a/src/i830_driver.c
> +++ b/src/i830_driver.c
> @@ -374,11 +374,13 @@ Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp)
> 8192,
> 0
> };
> + int pitch;
> int i;
>
> + pitch = *width * cpp;
> for (i = 0; pitches[i] != 0; i++) {
> - if (pitches[i] >= *width) {
> - *width = pitches[i];
> + if (pitches[i] >= pitch) {
> + *width = pitches[i] / cpp;
> tiled = TRUE;
> break;
> }
> @@ -389,14 +391,6 @@ Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp)
> }
>
> /*
> - * Pad to accelerator requirement
> - */
> -int i830_pad_drawable_width(int width, int cpp)
> -{
> - return (width + 63) & ~63;
> -}
> -
> -/*
> * DRM mode setting Linux only at this point... later on we could
> * add a wrapper here.
> */
> @@ -1100,8 +1094,7 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen, int argc, char **argv)
> struct pci_device *const device = intel->PciInfo;
> int fb_bar = IS_I9XX(intel) ? 2 : 0;
>
> - scrn->displayWidth =
> - i830_pad_drawable_width(scrn->virtualX, intel->cpp);
> + scrn->displayWidth = i830_pad_drawable_width(scrn->virtualX);
>
> /*
> * The "VideoRam" config file parameter specifies the maximum amount of
> --
> 1.7.0.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Acked-By: Matt Turner <mattst88 at gmail.com>
... in principle.
Carl said [1] that he's got two patches of mine in a branch waiting to
go it after 2.11. (I guess it's time now.) The patch in question is
here [2]/
[1] http://lists.freedesktop.org/archives/intel-gfx/2010-March/006272.html
[2] http://lists.freedesktop.org/archives/intel-gfx/2010-March/006244.html
Matt
More information about the Intel-gfx
mailing list