On Thu, Nov 3, 2016 at 10:12 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Wed, Nov 02, 2016 at 10:27:44AM -0400, Rob Clark wrote:
>> drm-hwc + android tries to create an fb for the wallpaper layer, which
>> is larger than the screen resolution, and potentially larger than
>> mode_config->max_{width,height}. But the plane src_w/src_h is within
>> the max limits, so it is something the hardware can otherwise do.
>>
>> For atomic drivers, defer the check to drm_atomic_plane_check().
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>> drivers/gpu/drm/drm_atomic.c | 17 +++++++++++++++++
>> drivers/gpu/drm/drm_crtc.c | 26 +++++++++++++++++---------
>> 2 files changed, 34 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 34edd4f..fb0f07ce 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -846,7 +846,9 @@ plane_switching_crtc(struct drm_atomic_state *state,
>> static int drm_atomic_plane_check(struct drm_plane *plane,
>> struct drm_plane_state *state)
>> {
>> + struct drm_mode_config *config = &plane->dev->mode_config;
>> unsigned int fb_width, fb_height;
>> + unsigned int min_width, max_width, min_height, max_height;
>> int ret;
>>
>> /* either *both* CRTC and FB must be set, or neither */
>> @@ -909,6 +911,21 @@ static int drm_atomic_plane_check(struct drm_plane
>> *plane,
>> return -ENOSPC;
>> }
>>
>> + min_width = config->min_width << 16;
>> + max_width = config->max_width << 16;
>> + min_height = config->min_height << 16;
>> + max_height = config->max_height << 16;
>> +
>> + /* Make sure source dimensions are within bounds. */
>> + if (min_width > state->src_w || state->src_w > max_width ||
>> + min_height > state->src_h || state->src_h > max_height) {
>> + DRM_DEBUG_ATOMIC("Invalid source size "
>> + "%u.%06ux%u.%06u\n",
>> + state->src_w >> 16, ((state->src_w & 0xffff)
>> * 15625) >> 10,
>> + state->src_h >> 16, ((state->src_h & 0xffff)
>> * 15625) >> 10);
>> + return -ERANGE;
>> + }
>> +
>> if (plane_switching_crtc(state->state, plane, state)) {
>> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>> plane->base.id, plane->name);
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index b4b973f..7294bde 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -3369,15 +3369,23 @@ internal_framebuffer_create(struct drm_device *dev,
>> return ERR_PTR(-EINVAL);
>> }
>>
>> - if ((config->min_width > r->width) || (r->width > config->max_width)) {
>> - DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <=
>> %d\n",
>> - r->width, config->min_width, config->max_width);
>> - return ERR_PTR(-EINVAL);
>> - }
>> - if ((config->min_height > r->height) || (r->height >
>> config->max_height)) {
>> - DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d &&
>> <= %d\n",
>> - r->height, config->min_height, config->max_height);
>> - return ERR_PTR(-EINVAL);
>> + /* for atomic drivers, we check the src dimensions in
>> + * drm_atomic_plane_check().. allow creation of a fb
>> + * that is larger than what can be scanned out, as
>> + * long as userspace doesn't try to scanout a portion
>> + * of the fb that is too large.
>> + */
>> + if (!file_priv->atomic) {
>> + if ((config->min_width > r->width) || (r->width >
>> config->max_width)) {
>> + DRM_DEBUG_KMS("bad framebuffer width %d, should be >=
>> %d && <= %d\n",
>> + r->width, config->min_width,
>> config->max_width);
>> + return ERR_PTR(-EINVAL);
>> + }
>> + if ((config->min_height > r->height) || (r->height >
>> config->max_height)) {
>> + DRM_DEBUG_KMS("bad framebuffer height %d, should be >=
>> %d && <= %d\n",
>> + r->height, config->min_height,
>> config->max_height);
>> + return ERR_PTR(-EINVAL);
>> + }
>
> So why not just bump max_foo in your driver?
>
> Removing the restriction from the core seems likely to break some
> drivers as they would now have to check the fb dimensions themselves.
that is why I did it only for atomic drivers, so we could rely on the
checking in drm_atomic_plane_check()..
BR,
-R
>> }
>>
>> if (r->flags & DRM_MODE_FB_MODIFIERS &&
>> --
>> 2.7.4
>
> --
> Ville Syrjälä
> Intel OTC
