This was all kicked off by me figuring out how the PPC byteswap thing worked. In the end, we're keeping the implicit byteswap based on architecture, but also expose the correctly supported formats, and fix some overlay details. Overlay framebuffers have various funky requirements that should be enforced at framebuffer creation time (these are set apart by their YUV formats). Further, I simplify some error-checking details for both nv4 and nv10+ framebuffers as well as using the pitch properly instead of making wild assumptions (how did that every work... not sure). This has seen some modest modetest testing on NV5, NV17, NV34, and NV4A. I noticed some odd issues on NV5 with certain scaling factors, but I'm somewhat sure that those issues were there before - either the hw can't keep up or we're not twiddling some clocks. Ilia Mirkin (4): drm/nouveau/display: prevent undisplayable framebuffers from creation drm/nouveau/overlay: improve error detection, fix pitch setting drm/nouveau/overlay: add NV21 support drm/nouveau/dispnv04: fix exposed format list drivers/gpu/drm/nouveau/dispnv04/crtc.c | 36 ++++++++++++++- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 71 +++++++++++++++++------------- drivers/gpu/drm/nouveau/nouveau_display.c | 21 +++++++++ 3 files changed, 96 insertions(+), 32 deletions(-) -- 2.13.0
Ilia Mirkin
2017-Aug-06 02:25 UTC
[Nouveau] [PATCH 1/4] drm/nouveau/display: prevent undisplayable framebuffers from creation
Pre-nv50 YUV overlays have stringent requirements for working with the internal machinery. Instead of rejecting these at update_plane time, we should instead prevent the framebuffers from being created in the first place. Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu> --- drivers/gpu/drm/nouveau/nouveau_display.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 549763f5e17d..99ae77f66177 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -256,9 +256,30 @@ nouveau_framebuffer_new(struct drm_device *dev, struct nouveau_bo *nvbo, struct nouveau_framebuffer **pfb) { + struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_framebuffer *fb; int ret; + /* YUV overlays have special requirements pre-NV50 */ + if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA && + + (mode_cmd->pixel_format == DRM_FORMAT_YUYV || + mode_cmd->pixel_format == DRM_FORMAT_UYVY || + mode_cmd->pixel_format == DRM_FORMAT_NV12 || + mode_cmd->pixel_format == DRM_FORMAT_NV21) && + (mode_cmd->pitches[0] & 0x3f || /* align 64 */ + mode_cmd->pitches[0] >= 0x10000 || /* at most 64k pitch */ + (mode_cmd->pitches[1] && /* pitches for planes must match */ + mode_cmd->pitches[0] != mode_cmd->pitches[1]))) { + struct drm_format_name_buf format_name; + DRM_DEBUG_KMS("Unsuitable framebuffer: format: %s; pitches: 0x%x\n 0x%x\n", + drm_get_format_name(mode_cmd->pixel_format, + &format_name), + mode_cmd->pitches[0], + mode_cmd->pitches[1]); + return -EINVAL; + } + if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL))) return -ENOMEM; -- 2.13.0
Ilia Mirkin
2017-Aug-06 02:25 UTC
[Nouveau] [PATCH 2/4] drm/nouveau/overlay: improve error detection, fix pitch setting
We were previously setting the pitch based on a perfectly packed buffer. This does not necessarily happen. Either modetest started generating such buffers recently, or earlier testing only happened with well-picked overlay sizes. While we're at it, beef up and refactor the error state detection. Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu> --- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 62 ++++++++++++++++-------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c index e54944d23268..5bd63c2f14a6 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c @@ -90,6 +90,26 @@ cos_mul(int degrees, int factor) } static int +verify_scaling(const struct drm_framebuffer *fb, uint8_t shift, + uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, + uint32_t crtc_w, uint32_t crtc_h) +{ + if (crtc_w < (src_w >> shift) || crtc_h < (src_h >> shift)) { + DRM_DEBUG_KMS("Unsuitable framebuffer scaling: %dx%d -> %dx%d\n", + src_w, src_h, crtc_w, crtc_h); + return -ERANGE; + } + + if (src_x != 0 || src_y != 0) { + DRM_DEBUG_KMS("Unsuitable framebuffer offset: %d,%d\n", + src_x, src_y); + return -ERANGE; + } + + return 0; +} + +static int nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, int crtc_x, int crtc_y, unsigned int crtc_w, unsigned int crtc_h, @@ -107,7 +127,9 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool flip = nv_plane->flip; int soff = NV_PCRTC0_SIZE * nv_crtc->index; int soff2 = NV_PCRTC0_SIZE * !nv_crtc->index; - int format, ret; + unsigned shift = drm->client.device.info.chipset >= 0x30 ? 1 : 3; + unsigned format = 0; + int ret; /* Source parameters given in 16.16 fixed point, ignore fractional. */ src_x >>= 16; @@ -115,18 +137,9 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, src_w >>= 16; src_h >>= 16; - format = ALIGN(src_w * 4, 0x100); - - if (format > 0xffff) - return -ERANGE; - - if (drm->client.device.info.chipset >= 0x30) { - if (crtc_w < (src_w >> 1) || crtc_h < (src_h >> 1)) - return -ERANGE; - } else { - if (crtc_w < (src_w >> 3) || crtc_h < (src_h >> 3)) - return -ERANGE; - } + ret = verify_scaling(fb, shift, 0, 0, src_w, src_h, crtc_w, crtc_h); + if (ret) + return ret; ret = nouveau_bo_pin(nv_fb->nvbo, TTM_PL_FLAG_VRAM, false); if (ret) @@ -160,7 +173,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset + fb->offsets[1]); } - nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format); + nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]); nvif_wr32(dev, NV_PVIDEO_STOP, 0); /* TODO: wait for vblank? */ nvif_wr32(dev, NV_PVIDEO_BUFFER, flip ? 0x10 : 0x1); @@ -357,7 +370,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct nouveau_bo *cur = nv_plane->cur; uint32_t overlay = 1; int brightness = (nv_plane->brightness - 512) * 62 / 512; - int pitch, ret, i; + int ret, i; /* Source parameters given in 16.16 fixed point, ignore fractional. */ src_x >>= 16; @@ -365,17 +378,9 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, src_w >>= 16; src_h >>= 16; - pitch = ALIGN(src_w * 4, 0x100); - - if (pitch > 0xffff) - return -ERANGE; - - /* TODO: Compute an offset? Not sure how to do this for YUYV. */ - if (src_x != 0 || src_y != 0) - return -ERANGE; - - if (crtc_w < src_w || crtc_h < src_h) - return -ERANGE; + ret = verify_scaling(fb, 0, src_x, src_y, src_w, src_h, crtc_w, crtc_h); + if (ret) + return ret; ret = nouveau_bo_pin(nv_fb->nvbo, TTM_PL_FLAG_VRAM, false); if (ret) @@ -389,8 +394,9 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, for (i = 0; i < 2; i++) { nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i, - nv_fb->nvbo->bo.offset); - nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i, pitch); + nv_fb->nvbo->bo.offset); + nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i, + fb->pitches[0]); nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 * i, 0); } nvif_wr32(dev, NV_PVIDEO_WINDOW_START, crtc_y << 16 | crtc_x); -- 2.13.0
Ilia Mirkin
2017-Aug-06 02:25 UTC
[Nouveau] [PATCH 3/4] drm/nouveau/overlay: add NV21 support
Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu> --- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c index 5bd63c2f14a6..c8c2333f24ee 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c @@ -63,6 +63,7 @@ static uint32_t formats[] = { DRM_FORMAT_YUYV, DRM_FORMAT_UYVY, DRM_FORMAT_NV12, + DRM_FORMAT_NV21, }; /* Sine can be approximated with @@ -159,16 +160,18 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, nvif_wr32(dev, NV_PVIDEO_POINT_OUT(flip), crtc_y << 16 | crtc_x); nvif_wr32(dev, NV_PVIDEO_SIZE_OUT(flip), crtc_h << 16 | crtc_w); - if (fb->format->format != DRM_FORMAT_UYVY) + if (fb->format->format == DRM_FORMAT_YUYV || + fb->format->format == DRM_FORMAT_NV12) format |= NV_PVIDEO_FORMAT_COLOR_LE_CR8YB8CB8YA8; - if (fb->format->format == DRM_FORMAT_NV12) + if (fb->format->format == DRM_FORMAT_NV12 || + fb->format->format == DRM_FORMAT_NV21) format |= NV_PVIDEO_FORMAT_PLANAR; if (nv_plane->iturbt_709) format |= NV_PVIDEO_FORMAT_MATRIX_ITURBT709; if (nv_plane->colorkey & (1 << 24)) format |= NV_PVIDEO_FORMAT_DISPLAY_COLOR_KEY; - if (fb->format->format == DRM_FORMAT_NV12) { + if (format & NV_PVIDEO_FORMAT_PLANAR) { nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0); nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset + fb->offsets[1]); -- 2.13.0
Ilia Mirkin
2017-Aug-06 02:25 UTC
[Nouveau] [PATCH 4/4] drm/nouveau/dispnv04: fix exposed format list
drm_crtc_init exposes the XRGB8888 and ARGB8888 formats. In actuality, ARGB8888's 32-bit depth messes up some formulas that weren't meant for it, and the alpha is fairly meaningless for the primary plane. The modesetting logic appears to be fully prepared for RGB565 as well as XRGB1555 however, as tested with modetest. Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu> --- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c index 4b4b0b496262..c078811b4e11 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c @@ -1099,6 +1099,38 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = { .disable = nv_crtc_disable, }; +static const uint32_t modeset_formats[] = { + DRM_FORMAT_XRGB8888, + DRM_FORMAT_RGB565, + DRM_FORMAT_XRGB1555, +}; + +static struct drm_plane * +create_primary_plane(struct drm_device *dev) +{ + struct drm_plane *primary; + int ret; + + primary = kzalloc(sizeof(*primary), GFP_KERNEL); + if (primary == NULL) { + DRM_DEBUG_KMS("Failed to allocate primary plane\n"); + return NULL; + } + + /* possible_crtc's will be filled in later by crtc_init */ + ret = drm_universal_plane_init(dev, primary, 0, + &drm_primary_helper_funcs, + modeset_formats, + ARRAY_SIZE(modeset_formats), + DRM_PLANE_TYPE_PRIMARY, NULL); + if (ret) { + kfree(primary); + primary = NULL; + } + + return primary; +} + int nv04_crtc_create(struct drm_device *dev, int crtc_num) { @@ -1122,7 +1154,9 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num) nv_crtc->save = nv_crtc_save; nv_crtc->restore = nv_crtc_restore; - drm_crtc_init(dev, &nv_crtc->base, &nv04_crtc_funcs); + drm_crtc_init_with_planes(dev, &nv_crtc->base, + create_primary_plane(dev), NULL, + &nv04_crtc_funcs, NULL); drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs); drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256); -- 2.13.0
Maybe Matching Threads
- [PATCH] drm/nouveau: Replace the iturbt_709 prop with the standarad COLOR_ENCODNIG prop
- [RFC PATCH 0/3] drm/nouveau/dispnv04 overlay and primary fb format fixes
- [RFC PATCH 0/3] drm/nouveau/dispnv04 overlay and primary fb format fixes
- [PATCH 1/5] drm/nv10/plane: fix format computation
- [PATCH] drm/nv10/plane: add plane support for nv10-nv40