[RFC] checking drm_framebuffer against config width/height

[RFC] checking drm_framebuffer against config width/height

[jsanka-sgV2jX0FEOL9JmXXK+q4OQ]

[-- Attachment #1.1: Type: text/plain, Size: 1869 bytes --]

Hello All, 

I bumped into the below check [1] enforced in drm_framebuffer creation which
checks the requested framebuffer width/height parameters against the drm
mode config width/height limits. As I understand, drm_mode_config: min/max
width/height indicate the upper and lower bounds of the display panel
(drm_connector) resolutions the DRM device can support. But the pixel
processing pipeline can apply cropping/scaling transformations on much
larger input framebuffers and flip the buffers within the display
resolution. Such configurations are very common and the final resolution
will be still within drm_mode_config bounds. So I believe the checks should
be relaxed / removed from the drm_framebuffer creation api's. 

 

If my understanding is incorrect, could somehow explain the motivation
behind having these checks here?

 

Thanks and Regards,

Jeykumar S.

 

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv
ers/gpu/drm/drm_framebuffer.c?h=v5.3#n303

 

struct drm_framebuffer *
drm_internal_framebuffer_create(struct drm_device *dev,
                               const struct drm_mode_fb_cmd2 *r,
                               struct drm_file *file_priv)
{

                     /* snip */

 

        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);

        }

        /* snip */

}

 


[-- Attachment #1.2: Type: text/html, Size: 10149 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [RFC] checking drm_framebuffer against config width/height

[Neil Armstrong]

Hi,

On 19/09/2019 05:30, jsanka@codeaurora.org wrote:
> Hello All,
> 
> I bumped into the below check [1] enforced in drm_framebuffer creation which checks the requested framebuffer width/height parameters against the drm mode config width/height limits. As I understand, drm_mode_config: min/max width/height indicate the upper and lower bounds of the display panel (drm_connector) resolutions the DRM device can support. But the pixel processing pipeline can apply cropping/scaling transformations on much larger input framebuffers and flip the buffers within the display resolution. Such configurations are very common and the final resolution will be still within drm_mode_config bounds. So I believe the checks should be relaxed / removed from the drm_framebuffer creation api’s.
> 
>  
> 
> If my understanding is incorrect, could somehow explain the motivation behind having these checks here?

I already reported this a few months ago, here is the log of the discussion :
https://patchwork.kernel.org/patch/10625707/

TL;DR it should be removed but these checks should be moved to control max screen size
in atomic_commit paths.

I planned to fix this, but haven't found time yet...

Neil

> 
>  
> 
> Thanks and Regards,
> 
> Jeykumar S.
> 
>  
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_framebuffer.c?h=v5.3#n303
> 
>  
> 
> *struct*drm_framebuffer *
> 
> *drm_internal_framebuffer_create*(*struct*drm_device *dev,
> 
>                                *const**struct*drm_mode_fb_cmd2 *r,
> 
>                                *struct*drm_file *file_priv)
> 
> {
> 
>                      /* snip */
> 
>  
> 
>         *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);
> 
>         }
> 
>         /* snip */
> 
> }
> 
>  
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [RFC] checking drm_framebuffer against config width/height

[Neil Armstrong]

Hi,

On 19/09/2019 05:30, jsanka@codeaurora.org wrote:
> Hello All,
> 
> I bumped into the below check [1] enforced in drm_framebuffer creation which checks the requested framebuffer width/height parameters against the drm mode config width/height limits. As I understand, drm_mode_config: min/max width/height indicate the upper and lower bounds of the display panel (drm_connector) resolutions the DRM device can support. But the pixel processing pipeline can apply cropping/scaling transformations on much larger input framebuffers and flip the buffers within the display resolution. Such configurations are very common and the final resolution will be still within drm_mode_config bounds. So I believe the checks should be relaxed / removed from the drm_framebuffer creation api’s.
> 
>  
> 
> If my understanding is incorrect, could somehow explain the motivation behind having these checks here?

I already reported this a few months ago, here is the log of the discussion :
https://patchwork.kernel.org/patch/10625707/

TL;DR it should be removed but these checks should be moved to control max screen size
in atomic_commit paths.

I planned to fix this, but haven't found time yet...

Neil

> 
>  
> 
> Thanks and Regards,
> 
> Jeykumar S.
> 
>  
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_framebuffer.c?h=v5.3#n303
> 
>  
> 
> *struct*drm_framebuffer *
> 
> *drm_internal_framebuffer_create*(*struct*drm_device *dev,
> 
>                                *const**struct*drm_mode_fb_cmd2 *r,
> 
>                                *struct*drm_file *file_priv)
> 
> {
> 
>                      /* snip */
> 
>  
> 
>         *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);
> 
>         }
> 
>         /* snip */
> 
> }
> 
>  
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno