On Wed, Aug 22, 2018 at 10:51 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@xxxxxxxxxx> wrote: >> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote: >>> Hey Noralf, all, >>> I've been digging for a bit on the regression that this patch has >>> tripped on the HiKey board as reported here: >>> https://lkml.org/lkml/2018/8/16/81 >>> >>> The first issue was that the kirin driver was setting >>> mode_config.max_width/height = 2048, which was causing errors as the >>> the requested resolution was 1920x2160 (due to surfaceflinger >>> requesting y*2 for page flipping). >> >> Hey Noralf, >> Sorry, I know your probably sick of me. But I just wanted to circle >> around on this little bit. So part of the issue I found earlier, was >> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support >> Surfaceflinger's request for page flipping. This is what makes the Y >> resolution 2160, which runs afoul of the new max_height check of 2048 >> in the generic code. >> >> I was checking with Xinliang, who know the kirin display hardware, >> about the max_height being set to 2048 to ensure bumping it up wasn't >> a problem, but he said 2048x2048 was unfortunately not arbitrary, and >> that was the hard limit of the display hardware. However, with >> overalloc, the 1920x2160 res fbdev should still be ok, as only >> 1920x1080 is actually displayed at one time. >> >> So it seems like we might need to multiply the max_height by the >> overalloc factor when we are checking it in >> drm_internal_framebuffer_create? >> >> Does that approach sound sane, or would folks prefer something different? > > I guess we could simply not check against the height limit when > allocating framebuffers. But we've done that for userspace buffers > since forever (they just allocate 2 buffers for page-flipping), so I > have no idea what would all break if we'd suddenly lift this > restriction. And whether we'd lift it for fbdev only or for everyone > doesn't really make much of a difference, since either this works, or > it doesn't (across all chips). That feels a bit more risky then what I was thinking. What about something like (apologies, whitespace corrupted) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index fe7e545..0424a71 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1810,6 +1810,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, int i; struct drm_fb_helper_surface_size sizes; int gamma_size = 0; + struct drm_mode_config *config; memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size)); sizes.surface_depth = 24; @@ -1910,6 +1911,11 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, sizes.surface_height *= drm_fbdev_overalloc; sizes.surface_height /= 100; + config = &fb_helper->client.dev->mode_config; + config->max_height *= drm_fbdev_overalloc; + config->max_height /= 100; + + /* push down into drivers */ ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes); if (ret < 0) That way it only effects the fbdev + overalloc case? thanks -john _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
