Den 23.08.2018 09.37, skrev Daniel Vetter:
On Wed, Aug 22, 2018 at 11:21:11PM -0700, John Stultz wrote:
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?
Still changes it for everyone, not just fbdev, if you enable overalloc.
You'd need to reset.
Another, cleaner way to fix this would be to overallocate the buffer, but
have the drm_framebuffer limited. But that means we need to change the
fbdev scrolling logic. And the entire interface between fbdev helpers and
drivers needs a rework, since atm the driver allocates the drm_framebuffer
for you. That's what userspace can/will do in this case I guess. Has all
the issues of scrolling by moving the drm_fb without hw knowledge.
I guess maybe just dropping the max_height check in fbdev is ok, if we put
a really big comment/FIXME there. Or maybe make it conditional on
fbdev_overalloc being at the default value, that'd probably be better
even.
Maybe something like this could work:
int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
struct drm_fb_helper_surface_size *sizes)
{
DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
sizes->surface_width, sizes->surface_height,
sizes->surface_bpp);
format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
if (drm_fbdev_overalloc > 100 &&
sizes->surface_height >
fb_helper->client.dev->mode_config.max_height) {
u32 divisor = drm_fbdev_overalloc / 100;
buffer = drm_client_buffer_create(client, sizes->surface_width,
sizes->surface_height, format);
if (IS_ERR(buffer))
return PTR_ERR(buffer);
ret = drm_client_buffer_addfb(buffer, sizes->surface_width,
sizes->surface_height / divisor, format);
if (ret) {
drm_client_buffer_delete(buffer);
return ret;
}
buffer->fb->height = sizes->surface_height;
} else {
buffer = drm_client_framebuffer_create(client,
sizes->surface_width,
sizes->surface_height, format);
if (IS_ERR(buffer))
return PTR_ERR(buffer);
}
Noralf.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
