* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-03 16:45 ` Eugeniy Paltsev 0 siblings, 0 replies; 15+ messages in thread From: Eugeniy Paltsev @ 2018-10-03 16:45 UTC (permalink / raw) To: linux-snps-arc drm fbdev emulation doesn't support changing the pixel format at all, so reject all pixel format changing requests. Cc: stable at vger.kernel.org Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com> --- Changes v1->v2: * Reject all pixel format changing request, not just the invalid ones. drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- 1 file changed, 26 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 16ec93b75dbf..48598d7f673f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, } EXPORT_SYMBOL(drm_fb_helper_ioctl); +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, + const struct fb_var_screeninfo *var_2) +{ + return var_1->bits_per_pixel == var_2->bits_per_pixel && + var_1->grayscale == var_2->grayscale && + var_1->red.offset == var_2->red.offset && + var_1->red.length == var_2->red.length && + var_1->red.msb_right == var_2->red.msb_right && + var_1->green.offset == var_2->green.offset && + var_1->green.length == var_2->green.length && + var_1->green.msb_right == var_2->green.msb_right && + var_1->blue.offset == var_2->blue.offset && + var_1->blue.length == var_2->blue.length && + var_1->blue.msb_right == var_2->blue.msb_right && + var_1->transp.offset == var_2->transp.offset && + var_1->transp.length == var_2->transp.length && + var_1->transp.msb_right == var_2->transp.msb_right; +} + /** * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var * @var: screeninfo to check @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, { struct drm_fb_helper *fb_helper = info->par; struct drm_framebuffer *fb = fb_helper->fb; - int depth; if (var->pixclock != 0 || in_dbg_master()) return -EINVAL; @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, return -EINVAL; } - switch (var->bits_per_pixel) { - case 16: - depth = (var->green.length == 6) ? 16 : 15; - break; - case 32: - depth = (var->transp.length > 0) ? 32 : 24; - break; - default: - depth = var->bits_per_pixel; - break; - } - - switch (depth) { - case 8: - var->red.offset = 0; - var->green.offset = 0; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 15: - var->red.offset = 10; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 5; - var->blue.length = 5; - var->transp.length = 1; - var->transp.offset = 15; - break; - case 16: - var->red.offset = 11; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 6; - var->blue.length = 5; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 24: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 32: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 8; - var->transp.offset = 24; - break; - default: + /* + * drm fbdev emulation doesn't support changing the pixel format at all, + * so reject all pixel format changing requests. + */ + if (!drm_fb_pixel_format_equal(var, &info->var)) { + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); return -EINVAL; } + return 0; } EXPORT_SYMBOL(drm_fb_helper_check_var); -- 2.14.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-03 16:45 ` Eugeniy Paltsev 0 siblings, 0 replies; 15+ messages in thread From: Eugeniy Paltsev @ 2018-10-03 16:45 UTC (permalink / raw) To: dri-devel Cc: linux-snps-arc, linux-kernel, Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, Alexey Brodkin, stable, Eugeniy Paltsev drm fbdev emulation doesn't support changing the pixel format at all, so reject all pixel format changing requests. Cc: stable@vger.kernel.org Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- Changes v1->v2: * Reject all pixel format changing request, not just the invalid ones. drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- 1 file changed, 26 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 16ec93b75dbf..48598d7f673f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, } EXPORT_SYMBOL(drm_fb_helper_ioctl); +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, + const struct fb_var_screeninfo *var_2) +{ + return var_1->bits_per_pixel == var_2->bits_per_pixel && + var_1->grayscale == var_2->grayscale && + var_1->red.offset == var_2->red.offset && + var_1->red.length == var_2->red.length && + var_1->red.msb_right == var_2->red.msb_right && + var_1->green.offset == var_2->green.offset && + var_1->green.length == var_2->green.length && + var_1->green.msb_right == var_2->green.msb_right && + var_1->blue.offset == var_2->blue.offset && + var_1->blue.length == var_2->blue.length && + var_1->blue.msb_right == var_2->blue.msb_right && + var_1->transp.offset == var_2->transp.offset && + var_1->transp.length == var_2->transp.length && + var_1->transp.msb_right == var_2->transp.msb_right; +} + /** * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var * @var: screeninfo to check @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, { struct drm_fb_helper *fb_helper = info->par; struct drm_framebuffer *fb = fb_helper->fb; - int depth; if (var->pixclock != 0 || in_dbg_master()) return -EINVAL; @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, return -EINVAL; } - switch (var->bits_per_pixel) { - case 16: - depth = (var->green.length == 6) ? 16 : 15; - break; - case 32: - depth = (var->transp.length > 0) ? 32 : 24; - break; - default: - depth = var->bits_per_pixel; - break; - } - - switch (depth) { - case 8: - var->red.offset = 0; - var->green.offset = 0; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 15: - var->red.offset = 10; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 5; - var->blue.length = 5; - var->transp.length = 1; - var->transp.offset = 15; - break; - case 16: - var->red.offset = 11; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 6; - var->blue.length = 5; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 24: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 32: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 8; - var->transp.offset = 24; - break; - default: + /* + * drm fbdev emulation doesn't support changing the pixel format at all, + * so reject all pixel format changing requests. + */ + if (!drm_fb_pixel_format_equal(var, &info->var)) { + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); return -EINVAL; } + return 0; } EXPORT_SYMBOL(drm_fb_helper_check_var); -- 2.14.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-03 16:45 ` Eugeniy Paltsev 0 siblings, 0 replies; 15+ messages in thread From: Eugeniy Paltsev @ 2018-10-03 16:45 UTC (permalink / raw) To: dri-devel Cc: David Airlie, Alexey Brodkin, linux-kernel, stable, Eugeniy Paltsev, linux-snps-arc, Sean Paul drm fbdev emulation doesn't support changing the pixel format at all, so reject all pixel format changing requests. Cc: stable@vger.kernel.org Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- Changes v1->v2: * Reject all pixel format changing request, not just the invalid ones. drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- 1 file changed, 26 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 16ec93b75dbf..48598d7f673f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, } EXPORT_SYMBOL(drm_fb_helper_ioctl); +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, + const struct fb_var_screeninfo *var_2) +{ + return var_1->bits_per_pixel == var_2->bits_per_pixel && + var_1->grayscale == var_2->grayscale && + var_1->red.offset == var_2->red.offset && + var_1->red.length == var_2->red.length && + var_1->red.msb_right == var_2->red.msb_right && + var_1->green.offset == var_2->green.offset && + var_1->green.length == var_2->green.length && + var_1->green.msb_right == var_2->green.msb_right && + var_1->blue.offset == var_2->blue.offset && + var_1->blue.length == var_2->blue.length && + var_1->blue.msb_right == var_2->blue.msb_right && + var_1->transp.offset == var_2->transp.offset && + var_1->transp.length == var_2->transp.length && + var_1->transp.msb_right == var_2->transp.msb_right; +} + /** * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var * @var: screeninfo to check @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, { struct drm_fb_helper *fb_helper = info->par; struct drm_framebuffer *fb = fb_helper->fb; - int depth; if (var->pixclock != 0 || in_dbg_master()) return -EINVAL; @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, return -EINVAL; } - switch (var->bits_per_pixel) { - case 16: - depth = (var->green.length == 6) ? 16 : 15; - break; - case 32: - depth = (var->transp.length > 0) ? 32 : 24; - break; - default: - depth = var->bits_per_pixel; - break; - } - - switch (depth) { - case 8: - var->red.offset = 0; - var->green.offset = 0; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 15: - var->red.offset = 10; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 5; - var->blue.length = 5; - var->transp.length = 1; - var->transp.offset = 15; - break; - case 16: - var->red.offset = 11; - var->green.offset = 5; - var->blue.offset = 0; - var->red.length = 5; - var->green.length = 6; - var->blue.length = 5; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 24: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 0; - var->transp.offset = 0; - break; - case 32: - var->red.offset = 16; - var->green.offset = 8; - var->blue.offset = 0; - var->red.length = 8; - var->green.length = 8; - var->blue.length = 8; - var->transp.length = 8; - var->transp.offset = 24; - break; - default: + /* + * drm fbdev emulation doesn't support changing the pixel format at all, + * so reject all pixel format changing requests. + */ + if (!drm_fb_pixel_format_equal(var, &info->var)) { + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); return -EINVAL; } + return 0; } EXPORT_SYMBOL(drm_fb_helper_check_var); -- 2.14.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests 2018-10-03 16:45 ` Eugeniy Paltsev @ 2018-10-04 7:56 ` Daniel Vetter -1 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-04 7:56 UTC (permalink / raw) To: linux-snps-arc On Wed, Oct 03, 2018@07:45:38PM +0300, Eugeniy Paltsev wrote: > drm fbdev emulation doesn't support changing the pixel format at all, > so reject all pixel format changing requests. For next time around: Please keep the note here why we need this and what the impact is. Otherwise it's not immediately clear why we should backport this patch to all stable kernels. I'll gather a few acks and then apply. -Daniel > > Cc: stable at vger.kernel.org > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com> > --- > Changes v1->v2: > * Reject all pixel format changing request, not just the invalid ones. > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 16ec93b75dbf..48598d7f673f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > } > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > + const struct fb_var_screeninfo *var_2) > +{ > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > + var_1->grayscale == var_2->grayscale && > + var_1->red.offset == var_2->red.offset && > + var_1->red.length == var_2->red.length && > + var_1->red.msb_right == var_2->red.msb_right && > + var_1->green.offset == var_2->green.offset && > + var_1->green.length == var_2->green.length && > + var_1->green.msb_right == var_2->green.msb_right && > + var_1->blue.offset == var_2->blue.offset && > + var_1->blue.length == var_2->blue.length && > + var_1->blue.msb_right == var_2->blue.msb_right && > + var_1->transp.offset == var_2->transp.offset && > + var_1->transp.length == var_2->transp.length && > + var_1->transp.msb_right == var_2->transp.msb_right; > +} > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > { > struct drm_fb_helper *fb_helper = info->par; > struct drm_framebuffer *fb = fb_helper->fb; > - int depth; > > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - switch (var->bits_per_pixel) { > - case 16: > - depth = (var->green.length == 6) ? 16 : 15; > - break; > - case 32: > - depth = (var->transp.length > 0) ? 32 : 24; > - break; > - default: > - depth = var->bits_per_pixel; > - break; > - } > - > - switch (depth) { > - case 8: > - var->red.offset = 0; > - var->green.offset = 0; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 15: > - var->red.offset = 10; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 5; > - var->blue.length = 5; > - var->transp.length = 1; > - var->transp.offset = 15; > - break; > - case 16: > - var->red.offset = 11; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 6; > - var->blue.length = 5; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 24: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 32: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 8; > - var->transp.offset = 24; > - break; > - default: > + /* > + * drm fbdev emulation doesn't support changing the pixel format at all, > + * so reject all pixel format changing requests. > + */ > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > return -EINVAL; > } > + > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_check_var); > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 7:56 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-04 7:56 UTC (permalink / raw) To: Eugeniy Paltsev Cc: dri-devel, David Airlie, Alexey Brodkin, linux-kernel, stable, linux-snps-arc, Sean Paul On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > drm fbdev emulation doesn't support changing the pixel format at all, > so reject all pixel format changing requests. For next time around: Please keep the note here why we need this and what the impact is. Otherwise it's not immediately clear why we should backport this patch to all stable kernels. I'll gather a few acks and then apply. -Daniel > > Cc: stable@vger.kernel.org > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > Changes v1->v2: > * Reject all pixel format changing request, not just the invalid ones. > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 16ec93b75dbf..48598d7f673f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > } > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > + const struct fb_var_screeninfo *var_2) > +{ > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > + var_1->grayscale == var_2->grayscale && > + var_1->red.offset == var_2->red.offset && > + var_1->red.length == var_2->red.length && > + var_1->red.msb_right == var_2->red.msb_right && > + var_1->green.offset == var_2->green.offset && > + var_1->green.length == var_2->green.length && > + var_1->green.msb_right == var_2->green.msb_right && > + var_1->blue.offset == var_2->blue.offset && > + var_1->blue.length == var_2->blue.length && > + var_1->blue.msb_right == var_2->blue.msb_right && > + var_1->transp.offset == var_2->transp.offset && > + var_1->transp.length == var_2->transp.length && > + var_1->transp.msb_right == var_2->transp.msb_right; > +} > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > { > struct drm_fb_helper *fb_helper = info->par; > struct drm_framebuffer *fb = fb_helper->fb; > - int depth; > > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - switch (var->bits_per_pixel) { > - case 16: > - depth = (var->green.length == 6) ? 16 : 15; > - break; > - case 32: > - depth = (var->transp.length > 0) ? 32 : 24; > - break; > - default: > - depth = var->bits_per_pixel; > - break; > - } > - > - switch (depth) { > - case 8: > - var->red.offset = 0; > - var->green.offset = 0; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 15: > - var->red.offset = 10; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 5; > - var->blue.length = 5; > - var->transp.length = 1; > - var->transp.offset = 15; > - break; > - case 16: > - var->red.offset = 11; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 6; > - var->blue.length = 5; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 24: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 32: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 8; > - var->transp.offset = 24; > - break; > - default: > + /* > + * drm fbdev emulation doesn't support changing the pixel format at all, > + * so reject all pixel format changing requests. > + */ > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > return -EINVAL; > } > + > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_check_var); > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests 2018-10-03 16:45 ` Eugeniy Paltsev (?) @ 2018-10-04 10:34 ` Ville Syrjälä -1 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 10:34 UTC (permalink / raw) To: linux-snps-arc On Wed, Oct 03, 2018@07:45:38PM +0300, Eugeniy Paltsev wrote: > drm fbdev emulation doesn't support changing the pixel format at all, > so reject all pixel format changing requests. > > Cc: stable at vger.kernel.org > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com> > --- > Changes v1->v2: > * Reject all pixel format changing request, not just the invalid ones. > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 16ec93b75dbf..48598d7f673f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > } > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > + const struct fb_var_screeninfo *var_2) > +{ > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > + var_1->grayscale == var_2->grayscale && > + var_1->red.offset == var_2->red.offset && > + var_1->red.length == var_2->red.length && > + var_1->red.msb_right == var_2->red.msb_right && > + var_1->green.offset == var_2->green.offset && > + var_1->green.length == var_2->green.length && > + var_1->green.msb_right == var_2->green.msb_right && > + var_1->blue.offset == var_2->blue.offset && > + var_1->blue.length == var_2->blue.length && > + var_1->blue.msb_right == var_2->blue.msb_right && > + var_1->transp.offset == var_2->transp.offset && > + var_1->transp.length == var_2->transp.length && > + var_1->transp.msb_right == var_2->transp.msb_right; > +} Could have shortened a bit with memcmp() but this works too. Reviewed-by: Ville Syrj?l? <ville.syrjala at linux.intel.com> I suppose we really should be doing the same for most of the rest of var_screeninfo as we don't allow changing the timings/etc. either. > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > { > struct drm_fb_helper *fb_helper = info->par; > struct drm_framebuffer *fb = fb_helper->fb; > - int depth; > > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - switch (var->bits_per_pixel) { > - case 16: > - depth = (var->green.length == 6) ? 16 : 15; > - break; > - case 32: > - depth = (var->transp.length > 0) ? 32 : 24; > - break; > - default: > - depth = var->bits_per_pixel; > - break; > - } > - > - switch (depth) { > - case 8: > - var->red.offset = 0; > - var->green.offset = 0; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 15: > - var->red.offset = 10; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 5; > - var->blue.length = 5; > - var->transp.length = 1; > - var->transp.offset = 15; > - break; > - case 16: > - var->red.offset = 11; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 6; > - var->blue.length = 5; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 24: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 32: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 8; > - var->transp.offset = 24; > - break; > - default: > + /* > + * drm fbdev emulation doesn't support changing the pixel format at all, > + * so reject all pixel format changing requests. > + */ > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > return -EINVAL; > } > + > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_check_var); > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj?l? Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 10:34 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 10:34 UTC (permalink / raw) To: Eugeniy Paltsev Cc: dri-devel, David Airlie, Alexey Brodkin, linux-kernel, stable, linux-snps-arc, Sean Paul On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > drm fbdev emulation doesn't support changing the pixel format at all, > so reject all pixel format changing requests. > > Cc: stable@vger.kernel.org > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > Changes v1->v2: > * Reject all pixel format changing request, not just the invalid ones. > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 16ec93b75dbf..48598d7f673f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > } > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > + const struct fb_var_screeninfo *var_2) > +{ > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > + var_1->grayscale == var_2->grayscale && > + var_1->red.offset == var_2->red.offset && > + var_1->red.length == var_2->red.length && > + var_1->red.msb_right == var_2->red.msb_right && > + var_1->green.offset == var_2->green.offset && > + var_1->green.length == var_2->green.length && > + var_1->green.msb_right == var_2->green.msb_right && > + var_1->blue.offset == var_2->blue.offset && > + var_1->blue.length == var_2->blue.length && > + var_1->blue.msb_right == var_2->blue.msb_right && > + var_1->transp.offset == var_2->transp.offset && > + var_1->transp.length == var_2->transp.length && > + var_1->transp.msb_right == var_2->transp.msb_right; > +} Could have shortened a bit with memcmp() but this works too. Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> I suppose we really should be doing the same for most of the rest of var_screeninfo as we don't allow changing the timings/etc. either. > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > { > struct drm_fb_helper *fb_helper = info->par; > struct drm_framebuffer *fb = fb_helper->fb; > - int depth; > > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - switch (var->bits_per_pixel) { > - case 16: > - depth = (var->green.length == 6) ? 16 : 15; > - break; > - case 32: > - depth = (var->transp.length > 0) ? 32 : 24; > - break; > - default: > - depth = var->bits_per_pixel; > - break; > - } > - > - switch (depth) { > - case 8: > - var->red.offset = 0; > - var->green.offset = 0; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 15: > - var->red.offset = 10; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 5; > - var->blue.length = 5; > - var->transp.length = 1; > - var->transp.offset = 15; > - break; > - case 16: > - var->red.offset = 11; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 6; > - var->blue.length = 5; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 24: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 32: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 8; > - var->transp.offset = 24; > - break; > - default: > + /* > + * drm fbdev emulation doesn't support changing the pixel format at all, > + * so reject all pixel format changing requests. > + */ > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > return -EINVAL; > } > + > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_check_var); > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj�l� Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 10:34 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 10:34 UTC (permalink / raw) To: Eugeniy Paltsev Cc: dri-devel, David Airlie, Alexey Brodkin, linux-kernel, stable, linux-snps-arc, Sean Paul On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > drm fbdev emulation doesn't support changing the pixel format at all, > so reject all pixel format changing requests. > > Cc: stable@vger.kernel.org > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > Changes v1->v2: > * Reject all pixel format changing request, not just the invalid ones. > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 16ec93b75dbf..48598d7f673f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > } > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > + const struct fb_var_screeninfo *var_2) > +{ > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > + var_1->grayscale == var_2->grayscale && > + var_1->red.offset == var_2->red.offset && > + var_1->red.length == var_2->red.length && > + var_1->red.msb_right == var_2->red.msb_right && > + var_1->green.offset == var_2->green.offset && > + var_1->green.length == var_2->green.length && > + var_1->green.msb_right == var_2->green.msb_right && > + var_1->blue.offset == var_2->blue.offset && > + var_1->blue.length == var_2->blue.length && > + var_1->blue.msb_right == var_2->blue.msb_right && > + var_1->transp.offset == var_2->transp.offset && > + var_1->transp.length == var_2->transp.length && > + var_1->transp.msb_right == var_2->transp.msb_right; > +} Could have shortened a bit with memcmp() but this works too. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I suppose we really should be doing the same for most of the rest of var_screeninfo as we don't allow changing the timings/etc. either. > + > /** > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > * @var: screeninfo to check > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > { > struct drm_fb_helper *fb_helper = info->par; > struct drm_framebuffer *fb = fb_helper->fb; > - int depth; > > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > return -EINVAL; > } > > - switch (var->bits_per_pixel) { > - case 16: > - depth = (var->green.length == 6) ? 16 : 15; > - break; > - case 32: > - depth = (var->transp.length > 0) ? 32 : 24; > - break; > - default: > - depth = var->bits_per_pixel; > - break; > - } > - > - switch (depth) { > - case 8: > - var->red.offset = 0; > - var->green.offset = 0; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 15: > - var->red.offset = 10; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 5; > - var->blue.length = 5; > - var->transp.length = 1; > - var->transp.offset = 15; > - break; > - case 16: > - var->red.offset = 11; > - var->green.offset = 5; > - var->blue.offset = 0; > - var->red.length = 5; > - var->green.length = 6; > - var->blue.length = 5; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 24: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 0; > - var->transp.offset = 0; > - break; > - case 32: > - var->red.offset = 16; > - var->green.offset = 8; > - var->blue.offset = 0; > - var->red.length = 8; > - var->green.length = 8; > - var->blue.length = 8; > - var->transp.length = 8; > - var->transp.offset = 24; > - break; > - default: > + /* > + * drm fbdev emulation doesn't support changing the pixel format at all, > + * so reject all pixel format changing requests. > + */ > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > return -EINVAL; > } > + > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_check_var); > -- > 2.14.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests 2018-10-04 10:34 ` Ville Syrjälä (?) @ 2018-10-04 10:49 ` Daniel Vetter -1 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-04 10:49 UTC (permalink / raw) To: linux-snps-arc On Thu, Oct 04, 2018@01:34:22PM +0300, Ville Syrj?l? wrote: > On Wed, Oct 03, 2018@07:45:38PM +0300, Eugeniy Paltsev wrote: > > drm fbdev emulation doesn't support changing the pixel format at all, > > so reject all pixel format changing requests. > > > > Cc: stable at vger.kernel.org > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com> > > --- > > Changes v1->v2: > > * Reject all pixel format changing request, not just the invalid ones. > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > index 16ec93b75dbf..48598d7f673f 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > } > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > + const struct fb_var_screeninfo *var_2) > > +{ > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > + var_1->grayscale == var_2->grayscale && > > + var_1->red.offset == var_2->red.offset && > > + var_1->red.length == var_2->red.length && > > + var_1->red.msb_right == var_2->red.msb_right && > > + var_1->green.offset == var_2->green.offset && > > + var_1->green.length == var_2->green.length && > > + var_1->green.msb_right == var_2->green.msb_right && > > + var_1->blue.offset == var_2->blue.offset && > > + var_1->blue.length == var_2->blue.length && > > + var_1->blue.msb_right == var_2->blue.msb_right && > > + var_1->transp.offset == var_2->transp.offset && > > + var_1->transp.length == var_2->transp.length && > > + var_1->transp.msb_right == var_2->transp.msb_right; > > +} > > Could have shortened a bit with memcmp() but this works too. I'm always vary of memcmp with structs that might have padding and stuff. > Reviewed-by: Ville Syrj?l? <ville.syrjala at linux.intel.com> > > I suppose we really should be doing the same for most of the rest of > var_screeninfo as we don't allow changing the timings/etc. either. Timing at least doens't have an immediate correctness impact of userspace rendering the wrong format :-) Thanks for review&patch, applied to drm-misc-fixes. -Daniel > > > + > > /** > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > * @var: screeninfo to check > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > { > > struct drm_fb_helper *fb_helper = info->par; > > struct drm_framebuffer *fb = fb_helper->fb; > > - int depth; > > > > if (var->pixclock != 0 || in_dbg_master()) > > return -EINVAL; > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > return -EINVAL; > > } > > > > - switch (var->bits_per_pixel) { > > - case 16: > > - depth = (var->green.length == 6) ? 16 : 15; > > - break; > > - case 32: > > - depth = (var->transp.length > 0) ? 32 : 24; > > - break; > > - default: > > - depth = var->bits_per_pixel; > > - break; > > - } > > - > > - switch (depth) { > > - case 8: > > - var->red.offset = 0; > > - var->green.offset = 0; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 15: > > - var->red.offset = 10; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 5; > > - var->blue.length = 5; > > - var->transp.length = 1; > > - var->transp.offset = 15; > > - break; > > - case 16: > > - var->red.offset = 11; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 6; > > - var->blue.length = 5; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 24: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 32: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 8; > > - var->transp.offset = 24; > > - break; > > - default: > > + /* > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > + * so reject all pixel format changing requests. > > + */ > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > return -EINVAL; > > } > > + > > return 0; > > } > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > -- > > 2.14.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrj?l? > Intel > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 10:49 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-04 10:49 UTC (permalink / raw) To: Ville Syrjälä Cc: Eugeniy Paltsev, David Airlie, Alexey Brodkin, linux-kernel, stable, dri-devel, linux-snps-arc, Sean Paul On Thu, Oct 04, 2018 at 01:34:22PM +0300, Ville Syrj�l� wrote: > On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > > drm fbdev emulation doesn't support changing the pixel format at all, > > so reject all pixel format changing requests. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > Changes v1->v2: > > * Reject all pixel format changing request, not just the invalid ones. > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > index 16ec93b75dbf..48598d7f673f 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > } > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > + const struct fb_var_screeninfo *var_2) > > +{ > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > + var_1->grayscale == var_2->grayscale && > > + var_1->red.offset == var_2->red.offset && > > + var_1->red.length == var_2->red.length && > > + var_1->red.msb_right == var_2->red.msb_right && > > + var_1->green.offset == var_2->green.offset && > > + var_1->green.length == var_2->green.length && > > + var_1->green.msb_right == var_2->green.msb_right && > > + var_1->blue.offset == var_2->blue.offset && > > + var_1->blue.length == var_2->blue.length && > > + var_1->blue.msb_right == var_2->blue.msb_right && > > + var_1->transp.offset == var_2->transp.offset && > > + var_1->transp.length == var_2->transp.length && > > + var_1->transp.msb_right == var_2->transp.msb_right; > > +} > > Could have shortened a bit with memcmp() but this works too. I'm always vary of memcmp with structs that might have padding and stuff. > Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > I suppose we really should be doing the same for most of the rest of > var_screeninfo as we don't allow changing the timings/etc. either. Timing at least doens't have an immediate correctness impact of userspace rendering the wrong format :-) Thanks for review&patch, applied to drm-misc-fixes. -Daniel > > > + > > /** > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > * @var: screeninfo to check > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > { > > struct drm_fb_helper *fb_helper = info->par; > > struct drm_framebuffer *fb = fb_helper->fb; > > - int depth; > > > > if (var->pixclock != 0 || in_dbg_master()) > > return -EINVAL; > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > return -EINVAL; > > } > > > > - switch (var->bits_per_pixel) { > > - case 16: > > - depth = (var->green.length == 6) ? 16 : 15; > > - break; > > - case 32: > > - depth = (var->transp.length > 0) ? 32 : 24; > > - break; > > - default: > > - depth = var->bits_per_pixel; > > - break; > > - } > > - > > - switch (depth) { > > - case 8: > > - var->red.offset = 0; > > - var->green.offset = 0; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 15: > > - var->red.offset = 10; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 5; > > - var->blue.length = 5; > > - var->transp.length = 1; > > - var->transp.offset = 15; > > - break; > > - case 16: > > - var->red.offset = 11; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 6; > > - var->blue.length = 5; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 24: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 32: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 8; > > - var->transp.offset = 24; > > - break; > > - default: > > + /* > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > + * so reject all pixel format changing requests. > > + */ > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > return -EINVAL; > > } > > + > > return 0; > > } > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > -- > > 2.14.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrj�l� > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 10:49 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-04 10:49 UTC (permalink / raw) To: Ville Syrjälä Cc: Eugeniy Paltsev, David Airlie, Alexey Brodkin, linux-kernel, stable, dri-devel, linux-snps-arc, Sean Paul On Thu, Oct 04, 2018 at 01:34:22PM +0300, Ville Syrjälä wrote: > On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > > drm fbdev emulation doesn't support changing the pixel format at all, > > so reject all pixel format changing requests. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > Changes v1->v2: > > * Reject all pixel format changing request, not just the invalid ones. > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > index 16ec93b75dbf..48598d7f673f 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > } > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > + const struct fb_var_screeninfo *var_2) > > +{ > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > + var_1->grayscale == var_2->grayscale && > > + var_1->red.offset == var_2->red.offset && > > + var_1->red.length == var_2->red.length && > > + var_1->red.msb_right == var_2->red.msb_right && > > + var_1->green.offset == var_2->green.offset && > > + var_1->green.length == var_2->green.length && > > + var_1->green.msb_right == var_2->green.msb_right && > > + var_1->blue.offset == var_2->blue.offset && > > + var_1->blue.length == var_2->blue.length && > > + var_1->blue.msb_right == var_2->blue.msb_right && > > + var_1->transp.offset == var_2->transp.offset && > > + var_1->transp.length == var_2->transp.length && > > + var_1->transp.msb_right == var_2->transp.msb_right; > > +} > > Could have shortened a bit with memcmp() but this works too. I'm always vary of memcmp with structs that might have padding and stuff. > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I suppose we really should be doing the same for most of the rest of > var_screeninfo as we don't allow changing the timings/etc. either. Timing at least doens't have an immediate correctness impact of userspace rendering the wrong format :-) Thanks for review&patch, applied to drm-misc-fixes. -Daniel > > > + > > /** > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > * @var: screeninfo to check > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > { > > struct drm_fb_helper *fb_helper = info->par; > > struct drm_framebuffer *fb = fb_helper->fb; > > - int depth; > > > > if (var->pixclock != 0 || in_dbg_master()) > > return -EINVAL; > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > return -EINVAL; > > } > > > > - switch (var->bits_per_pixel) { > > - case 16: > > - depth = (var->green.length == 6) ? 16 : 15; > > - break; > > - case 32: > > - depth = (var->transp.length > 0) ? 32 : 24; > > - break; > > - default: > > - depth = var->bits_per_pixel; > > - break; > > - } > > - > > - switch (depth) { > > - case 8: > > - var->red.offset = 0; > > - var->green.offset = 0; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 15: > > - var->red.offset = 10; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 5; > > - var->blue.length = 5; > > - var->transp.length = 1; > > - var->transp.offset = 15; > > - break; > > - case 16: > > - var->red.offset = 11; > > - var->green.offset = 5; > > - var->blue.offset = 0; > > - var->red.length = 5; > > - var->green.length = 6; > > - var->blue.length = 5; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 24: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 0; > > - var->transp.offset = 0; > > - break; > > - case 32: > > - var->red.offset = 16; > > - var->green.offset = 8; > > - var->blue.offset = 0; > > - var->red.length = 8; > > - var->green.length = 8; > > - var->blue.length = 8; > > - var->transp.length = 8; > > - var->transp.offset = 24; > > - break; > > - default: > > + /* > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > + * so reject all pixel format changing requests. > > + */ > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > return -EINVAL; > > } > > + > > return 0; > > } > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > -- > > 2.14.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm: fb-helper: Reject all pixel format changing requests 2018-10-04 10:49 ` Daniel Vetter (?) (?) @ 2018-10-04 11:00 ` Ville Syrjälä -1 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 11:00 UTC (permalink / raw) To: linux-snps-arc On Thu, Oct 04, 2018@12:49:38PM +0200, Daniel Vetter wrote: > On Thu, Oct 04, 2018@01:34:22PM +0300, Ville Syrj?l? wrote: > > On Wed, Oct 03, 2018@07:45:38PM +0300, Eugeniy Paltsev wrote: > > > drm fbdev emulation doesn't support changing the pixel format at all, > > > so reject all pixel format changing requests. > > > > > > Cc: stable at vger.kernel.org > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com> > > > --- > > > Changes v1->v2: > > > * Reject all pixel format changing request, not just the invalid ones. > > > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 16ec93b75dbf..48598d7f673f 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > > } > > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > > + const struct fb_var_screeninfo *var_2) > > > +{ > > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > > + var_1->grayscale == var_2->grayscale && > > > + var_1->red.offset == var_2->red.offset && > > > + var_1->red.length == var_2->red.length && > > > + var_1->red.msb_right == var_2->red.msb_right && > > > + var_1->green.offset == var_2->green.offset && > > > + var_1->green.length == var_2->green.length && > > > + var_1->green.msb_right == var_2->green.msb_right && > > > + var_1->blue.offset == var_2->blue.offset && > > > + var_1->blue.length == var_2->blue.length && > > > + var_1->blue.msb_right == var_2->blue.msb_right && > > > + var_1->transp.offset == var_2->transp.offset && > > > + var_1->transp.length == var_2->transp.length && > > > + var_1->transp.msb_right == var_2->transp.msb_right; > > > +} > > > > Could have shortened a bit with memcmp() but this works too. > > I'm always vary of memcmp with structs that might have padding and stuff. These are uabi so padding would be rather bad. > > > Reviewed-by: Ville Syrj?l? <ville.syrjala at linux.intel.com> > > > > I suppose we really should be doing the same for most of the rest of > > var_screeninfo as we don't allow changing the timings/etc. either. > > Timing at least doens't have an immediate correctness impact of userspace > rendering the wrong format :-) > > Thanks for review&patch, applied to drm-misc-fixes. > -Daniel > > > > > > + > > > /** > > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > > * @var: screeninfo to check > > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > { > > > struct drm_fb_helper *fb_helper = info->par; > > > struct drm_framebuffer *fb = fb_helper->fb; > > > - int depth; > > > > > > if (var->pixclock != 0 || in_dbg_master()) > > > return -EINVAL; > > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > return -EINVAL; > > > } > > > > > > - switch (var->bits_per_pixel) { > > > - case 16: > > > - depth = (var->green.length == 6) ? 16 : 15; > > > - break; > > > - case 32: > > > - depth = (var->transp.length > 0) ? 32 : 24; > > > - break; > > > - default: > > > - depth = var->bits_per_pixel; > > > - break; > > > - } > > > - > > > - switch (depth) { > > > - case 8: > > > - var->red.offset = 0; > > > - var->green.offset = 0; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 15: > > > - var->red.offset = 10; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 5; > > > - var->blue.length = 5; > > > - var->transp.length = 1; > > > - var->transp.offset = 15; > > > - break; > > > - case 16: > > > - var->red.offset = 11; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 6; > > > - var->blue.length = 5; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 24: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 32: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 8; > > > - var->transp.offset = 24; > > > - break; > > > - default: > > > + /* > > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > > + * so reject all pixel format changing requests. > > > + */ > > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > > return -EINVAL; > > > } > > > + > > > return 0; > > > } > > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > > -- > > > 2.14.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel at lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrj?l? > > Intel > > _______________________________________________ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrj?l? Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 11:00 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 11:00 UTC (permalink / raw) To: Eugeniy Paltsev, David Airlie, Alexey Brodkin, linux-kernel, stable, dri-devel, linux-snps-arc, Sean Paul On Thu, Oct 04, 2018 at 12:49:38PM +0200, Daniel Vetter wrote: > On Thu, Oct 04, 2018 at 01:34:22PM +0300, Ville Syrj�l� wrote: > > On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > > > drm fbdev emulation doesn't support changing the pixel format at all, > > > so reject all pixel format changing requests. > > > > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > > --- > > > Changes v1->v2: > > > * Reject all pixel format changing request, not just the invalid ones. > > > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 16ec93b75dbf..48598d7f673f 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > > } > > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > > + const struct fb_var_screeninfo *var_2) > > > +{ > > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > > + var_1->grayscale == var_2->grayscale && > > > + var_1->red.offset == var_2->red.offset && > > > + var_1->red.length == var_2->red.length && > > > + var_1->red.msb_right == var_2->red.msb_right && > > > + var_1->green.offset == var_2->green.offset && > > > + var_1->green.length == var_2->green.length && > > > + var_1->green.msb_right == var_2->green.msb_right && > > > + var_1->blue.offset == var_2->blue.offset && > > > + var_1->blue.length == var_2->blue.length && > > > + var_1->blue.msb_right == var_2->blue.msb_right && > > > + var_1->transp.offset == var_2->transp.offset && > > > + var_1->transp.length == var_2->transp.length && > > > + var_1->transp.msb_right == var_2->transp.msb_right; > > > +} > > > > Could have shortened a bit with memcmp() but this works too. > > I'm always vary of memcmp with structs that might have padding and stuff. These are uabi so padding would be rather bad. > > > Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > I suppose we really should be doing the same for most of the rest of > > var_screeninfo as we don't allow changing the timings/etc. either. > > Timing at least doens't have an immediate correctness impact of userspace > rendering the wrong format :-) > > Thanks for review&patch, applied to drm-misc-fixes. > -Daniel > > > > > > + > > > /** > > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > > * @var: screeninfo to check > > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > { > > > struct drm_fb_helper *fb_helper = info->par; > > > struct drm_framebuffer *fb = fb_helper->fb; > > > - int depth; > > > > > > if (var->pixclock != 0 || in_dbg_master()) > > > return -EINVAL; > > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > return -EINVAL; > > > } > > > > > > - switch (var->bits_per_pixel) { > > > - case 16: > > > - depth = (var->green.length == 6) ? 16 : 15; > > > - break; > > > - case 32: > > > - depth = (var->transp.length > 0) ? 32 : 24; > > > - break; > > > - default: > > > - depth = var->bits_per_pixel; > > > - break; > > > - } > > > - > > > - switch (depth) { > > > - case 8: > > > - var->red.offset = 0; > > > - var->green.offset = 0; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 15: > > > - var->red.offset = 10; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 5; > > > - var->blue.length = 5; > > > - var->transp.length = 1; > > > - var->transp.offset = 15; > > > - break; > > > - case 16: > > > - var->red.offset = 11; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 6; > > > - var->blue.length = 5; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 24: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 32: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 8; > > > - var->transp.offset = 24; > > > - break; > > > - default: > > > + /* > > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > > + * so reject all pixel format changing requests. > > > + */ > > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > > return -EINVAL; > > > } > > > + > > > return 0; > > > } > > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > > -- > > > 2.14.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrj�l� > > Intel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrj�l� Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 11:00 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 11:00 UTC (permalink / raw) To: Eugeniy Paltsev, David Airlie, Alexey Brodkin, linux-kernel, stable, dri-devel, linux-snps-arc, Sean Paul On Thu, Oct 04, 2018 at 12:49:38PM +0200, Daniel Vetter wrote: > On Thu, Oct 04, 2018 at 01:34:22PM +0300, Ville Syrjälä wrote: > > On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > > > drm fbdev emulation doesn't support changing the pixel format at all, > > > so reject all pixel format changing requests. > > > > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > > --- > > > Changes v1->v2: > > > * Reject all pixel format changing request, not just the invalid ones. > > > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 16ec93b75dbf..48598d7f673f 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > > } > > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > > + const struct fb_var_screeninfo *var_2) > > > +{ > > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > > + var_1->grayscale == var_2->grayscale && > > > + var_1->red.offset == var_2->red.offset && > > > + var_1->red.length == var_2->red.length && > > > + var_1->red.msb_right == var_2->red.msb_right && > > > + var_1->green.offset == var_2->green.offset && > > > + var_1->green.length == var_2->green.length && > > > + var_1->green.msb_right == var_2->green.msb_right && > > > + var_1->blue.offset == var_2->blue.offset && > > > + var_1->blue.length == var_2->blue.length && > > > + var_1->blue.msb_right == var_2->blue.msb_right && > > > + var_1->transp.offset == var_2->transp.offset && > > > + var_1->transp.length == var_2->transp.length && > > > + var_1->transp.msb_right == var_2->transp.msb_right; > > > +} > > > > Could have shortened a bit with memcmp() but this works too. > > I'm always vary of memcmp with structs that might have padding and stuff. These are uabi so padding would be rather bad. > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I suppose we really should be doing the same for most of the rest of > > var_screeninfo as we don't allow changing the timings/etc. either. > > Timing at least doens't have an immediate correctness impact of userspace > rendering the wrong format :-) > > Thanks for review&patch, applied to drm-misc-fixes. > -Daniel > > > > > > + > > > /** > > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > > * @var: screeninfo to check > > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > { > > > struct drm_fb_helper *fb_helper = info->par; > > > struct drm_framebuffer *fb = fb_helper->fb; > > > - int depth; > > > > > > if (var->pixclock != 0 || in_dbg_master()) > > > return -EINVAL; > > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > return -EINVAL; > > > } > > > > > > - switch (var->bits_per_pixel) { > > > - case 16: > > > - depth = (var->green.length == 6) ? 16 : 15; > > > - break; > > > - case 32: > > > - depth = (var->transp.length > 0) ? 32 : 24; > > > - break; > > > - default: > > > - depth = var->bits_per_pixel; > > > - break; > > > - } > > > - > > > - switch (depth) { > > > - case 8: > > > - var->red.offset = 0; > > > - var->green.offset = 0; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 15: > > > - var->red.offset = 10; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 5; > > > - var->blue.length = 5; > > > - var->transp.length = 1; > > > - var->transp.offset = 15; > > > - break; > > > - case 16: > > > - var->red.offset = 11; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 6; > > > - var->blue.length = 5; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 24: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 32: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 8; > > > - var->transp.offset = 24; > > > - break; > > > - default: > > > + /* > > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > > + * so reject all pixel format changing requests. > > > + */ > > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > > return -EINVAL; > > > } > > > + > > > return 0; > > > } > > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > > -- > > > 2.14.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests @ 2018-10-04 11:00 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2018-10-04 11:00 UTC (permalink / raw) To: Eugeniy Paltsev, David Airlie, Alexey Brodkin, linux-kernel, stable, dri-devel, linux-snps-arc, Sean Paul On Thu, Oct 04, 2018 at 12:49:38PM +0200, Daniel Vetter wrote: > On Thu, Oct 04, 2018 at 01:34:22PM +0300, Ville Syrjälä wrote: > > On Wed, Oct 03, 2018 at 07:45:38PM +0300, Eugeniy Paltsev wrote: > > > drm fbdev emulation doesn't support changing the pixel format at all, > > > so reject all pixel format changing requests. > > > > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > > --- > > > Changes v1->v2: > > > * Reject all pixel format changing request, not just the invalid ones. > > > > > > drivers/gpu/drm/drm_fb_helper.c | 91 ++++++++++++----------------------------- > > > 1 file changed, 26 insertions(+), 65 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 16ec93b75dbf..48598d7f673f 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -1580,6 +1580,25 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, > > > } > > > EXPORT_SYMBOL(drm_fb_helper_ioctl); > > > > > > +static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, > > > + const struct fb_var_screeninfo *var_2) > > > +{ > > > + return var_1->bits_per_pixel == var_2->bits_per_pixel && > > > + var_1->grayscale == var_2->grayscale && > > > + var_1->red.offset == var_2->red.offset && > > > + var_1->red.length == var_2->red.length && > > > + var_1->red.msb_right == var_2->red.msb_right && > > > + var_1->green.offset == var_2->green.offset && > > > + var_1->green.length == var_2->green.length && > > > + var_1->green.msb_right == var_2->green.msb_right && > > > + var_1->blue.offset == var_2->blue.offset && > > > + var_1->blue.length == var_2->blue.length && > > > + var_1->blue.msb_right == var_2->blue.msb_right && > > > + var_1->transp.offset == var_2->transp.offset && > > > + var_1->transp.length == var_2->transp.length && > > > + var_1->transp.msb_right == var_2->transp.msb_right; > > > +} > > > > Could have shortened a bit with memcmp() but this works too. > > I'm always vary of memcmp with structs that might have padding and stuff. These are uabi so padding would be rather bad. > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I suppose we really should be doing the same for most of the rest of > > var_screeninfo as we don't allow changing the timings/etc. either. > > Timing at least doens't have an immediate correctness impact of userspace > rendering the wrong format :-) > > Thanks for review&patch, applied to drm-misc-fixes. > -Daniel > > > > > > + > > > /** > > > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var > > > * @var: screeninfo to check > > > @@ -1590,7 +1609,6 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > { > > > struct drm_fb_helper *fb_helper = info->par; > > > struct drm_framebuffer *fb = fb_helper->fb; > > > - int depth; > > > > > > if (var->pixclock != 0 || in_dbg_master()) > > > return -EINVAL; > > > @@ -1610,72 +1628,15 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > > > return -EINVAL; > > > } > > > > > > - switch (var->bits_per_pixel) { > > > - case 16: > > > - depth = (var->green.length == 6) ? 16 : 15; > > > - break; > > > - case 32: > > > - depth = (var->transp.length > 0) ? 32 : 24; > > > - break; > > > - default: > > > - depth = var->bits_per_pixel; > > > - break; > > > - } > > > - > > > - switch (depth) { > > > - case 8: > > > - var->red.offset = 0; > > > - var->green.offset = 0; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 15: > > > - var->red.offset = 10; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 5; > > > - var->blue.length = 5; > > > - var->transp.length = 1; > > > - var->transp.offset = 15; > > > - break; > > > - case 16: > > > - var->red.offset = 11; > > > - var->green.offset = 5; > > > - var->blue.offset = 0; > > > - var->red.length = 5; > > > - var->green.length = 6; > > > - var->blue.length = 5; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 24: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 0; > > > - var->transp.offset = 0; > > > - break; > > > - case 32: > > > - var->red.offset = 16; > > > - var->green.offset = 8; > > > - var->blue.offset = 0; > > > - var->red.length = 8; > > > - var->green.length = 8; > > > - var->blue.length = 8; > > > - var->transp.length = 8; > > > - var->transp.offset = 24; > > > - break; > > > - default: > > > + /* > > > + * drm fbdev emulation doesn't support changing the pixel format at all, > > > + * so reject all pixel format changing requests. > > > + */ > > > + if (!drm_fb_pixel_format_equal(var, &info->var)) { > > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel format\n"); > > > return -EINVAL; > > > } > > > + > > > return 0; > > > } > > > EXPORT_SYMBOL(drm_fb_helper_check_var); > > > -- > > > 2.14.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-10-04 17:53 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-10-03 16:45 [PATCH v2] drm: fb-helper: Reject all pixel format changing requests Eugeniy Paltsev 2018-10-03 16:45 ` Eugeniy Paltsev 2018-10-03 16:45 ` Eugeniy Paltsev 2018-10-04 7:56 ` Daniel Vetter 2018-10-04 7:56 ` Daniel Vetter 2018-10-04 10:34 ` Ville Syrjälä 2018-10-04 10:34 ` Ville Syrjälä 2018-10-04 10:34 ` Ville Syrjälä 2018-10-04 10:49 ` Daniel Vetter 2018-10-04 10:49 ` Daniel Vetter 2018-10-04 10:49 ` Daniel Vetter 2018-10-04 11:00 ` Ville Syrjälä 2018-10-04 11:00 ` Ville Syrjälä 2018-10-04 11:00 ` Ville Syrjälä 2018-10-04 11:00 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.