From mboxrd@z Thu Jan 1 00:00:00 1970 From: ville.syrjala@linux.intel.com (Ville =?iso-8859-1?Q?Syrj=E4l=E4?=) Date: Thu, 4 Oct 2018 14:00:43 +0300 Subject: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests In-Reply-To: <20181004104938.GJ31561@phenom.ffwll.local> References: <20181003164538.5534-1-Eugeniy.Paltsev@synopsys.com> <20181004103422.GL9144@intel.com> <20181004104938.GJ31561@phenom.ffwll.local> List-ID: Message-ID: <20181004110043.GP9144@intel.com> To: linux-snps-arc@lists.infradead.org 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 > > > --- > > > 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? > > > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests Date: Thu, 4 Oct 2018 14:00:43 +0300 Message-ID: <20181004110043.GP9144@intel.com> References: <20181003164538.5534-1-Eugeniy.Paltsev@synopsys.com> <20181004103422.GL9144@intel.com> <20181004104938.GJ31561@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 42E4B6E5D1 for ; Thu, 4 Oct 2018 11:00:48 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20181004104938.GJ31561@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Eugeniy Paltsev , David Airlie , Alexey Brodkin , linux-kernel@vger.kernel.org, stable@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-snps-arc@lists.infradead.org, Sean Paul List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBPY3QgMDQsIDIwMTggYXQgMTI6NDk6MzhQTSArMDIwMCwgRGFuaWVsIFZldHRlciB3 cm90ZToKPiBPbiBUaHUsIE9jdCAwNCwgMjAxOCBhdCAwMTozNDoyMlBNICswMzAwLCBWaWxsZSBT eXJqw6Rsw6Qgd3JvdGU6Cj4gPiBPbiBXZWQsIE9jdCAwMywgMjAxOCBhdCAwNzo0NTozOFBNICsw MzAwLCBFdWdlbml5IFBhbHRzZXYgd3JvdGU6Cj4gPiA+IGRybSBmYmRldiBlbXVsYXRpb24gZG9l c24ndCBzdXBwb3J0IGNoYW5naW5nIHRoZSBwaXhlbCBmb3JtYXQgYXQgYWxsLAo+ID4gPiBzbyBy ZWplY3QgYWxsIHBpeGVsIGZvcm1hdCBjaGFuZ2luZyByZXF1ZXN0cy4KPiA+ID4gCj4gPiA+IENj OiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEV1Z2VuaXkgUGFs dHNldiA8RXVnZW5peS5QYWx0c2V2QHN5bm9wc3lzLmNvbT4KPiA+ID4gLS0tCj4gPiA+IENoYW5n ZXMgdjEtPnYyOgo+ID4gPiAgKiBSZWplY3QgYWxsIHBpeGVsIGZvcm1hdCBjaGFuZ2luZyByZXF1 ZXN0LCBub3QganVzdCB0aGUgaW52YWxpZCBvbmVzLgo+ID4gPiAKPiA+ID4gIGRyaXZlcnMvZ3B1 L2RybS9kcm1fZmJfaGVscGVyLmMgfCA5MSArKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLQo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDI2IGluc2VydGlvbnMoKyksIDY1IGRl bGV0aW9ucygtKQo+ID4gPiAKPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1f ZmJfaGVscGVyLmMgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2ZiX2hlbHBlci5jCj4gPiA+IGluZGV4 IDE2ZWM5M2I3NWRiZi4uNDg1OThkN2Y2NzNmIDEwMDY0NAo+ID4gPiAtLS0gYS9kcml2ZXJzL2dw dS9kcm0vZHJtX2ZiX2hlbHBlci5jCj4gPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJf aGVscGVyLmMKPiA+ID4gQEAgLTE1ODAsNiArMTU4MCwyNSBAQCBpbnQgZHJtX2ZiX2hlbHBlcl9p b2N0bChzdHJ1Y3QgZmJfaW5mbyAqaW5mbywgdW5zaWduZWQgaW50IGNtZCwKPiA+ID4gIH0KPiA+ ID4gIEVYUE9SVF9TWU1CT0woZHJtX2ZiX2hlbHBlcl9pb2N0bCk7Cj4gPiA+ICAKPiA+ID4gK3N0 YXRpYyBib29sIGRybV9mYl9waXhlbF9mb3JtYXRfZXF1YWwoY29uc3Qgc3RydWN0IGZiX3Zhcl9z Y3JlZW5pbmZvICp2YXJfMSwKPiA+ID4gKwkJCQkgICAgICBjb25zdCBzdHJ1Y3QgZmJfdmFyX3Nj cmVlbmluZm8gKnZhcl8yKQo+ID4gPiArewo+ID4gPiArCXJldHVybiB2YXJfMS0+Yml0c19wZXJf cGl4ZWwgPT0gdmFyXzItPmJpdHNfcGVyX3BpeGVsICYmCj4gPiA+ICsJICAgICAgIHZhcl8xLT5n cmF5c2NhbGUgPT0gdmFyXzItPmdyYXlzY2FsZSAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+cmVk Lm9mZnNldCA9PSB2YXJfMi0+cmVkLm9mZnNldCAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+cmVk Lmxlbmd0aCA9PSB2YXJfMi0+cmVkLmxlbmd0aCAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+cmVk Lm1zYl9yaWdodCA9PSB2YXJfMi0+cmVkLm1zYl9yaWdodCAmJgo+ID4gPiArCSAgICAgICB2YXJf MS0+Z3JlZW4ub2Zmc2V0ID09IHZhcl8yLT5ncmVlbi5vZmZzZXQgJiYKPiA+ID4gKwkgICAgICAg dmFyXzEtPmdyZWVuLmxlbmd0aCA9PSB2YXJfMi0+Z3JlZW4ubGVuZ3RoICYmCj4gPiA+ICsJICAg ICAgIHZhcl8xLT5ncmVlbi5tc2JfcmlnaHQgPT0gdmFyXzItPmdyZWVuLm1zYl9yaWdodCAmJgo+ ID4gPiArCSAgICAgICB2YXJfMS0+Ymx1ZS5vZmZzZXQgPT0gdmFyXzItPmJsdWUub2Zmc2V0ICYm Cj4gPiA+ICsJICAgICAgIHZhcl8xLT5ibHVlLmxlbmd0aCA9PSB2YXJfMi0+Ymx1ZS5sZW5ndGgg JiYKPiA+ID4gKwkgICAgICAgdmFyXzEtPmJsdWUubXNiX3JpZ2h0ID09IHZhcl8yLT5ibHVlLm1z Yl9yaWdodCAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+dHJhbnNwLm9mZnNldCA9PSB2YXJfMi0+ dHJhbnNwLm9mZnNldCAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+dHJhbnNwLmxlbmd0aCA9PSB2 YXJfMi0+dHJhbnNwLmxlbmd0aCAmJgo+ID4gPiArCSAgICAgICB2YXJfMS0+dHJhbnNwLm1zYl9y aWdodCA9PSB2YXJfMi0+dHJhbnNwLm1zYl9yaWdodDsKPiA+ID4gK30KPiA+IAo+ID4gQ291bGQg aGF2ZSBzaG9ydGVuZWQgYSBiaXQgd2l0aCBtZW1jbXAoKSBidXQgdGhpcyB3b3JrcyB0b28uCj4g Cj4gSSdtIGFsd2F5cyB2YXJ5IG9mIG1lbWNtcCB3aXRoIHN0cnVjdHMgdGhhdCBtaWdodCBoYXZl IHBhZGRpbmcgYW5kIHN0dWZmLgoKVGhlc2UgYXJlIHVhYmkgc28gcGFkZGluZyB3b3VsZCBiZSBy YXRoZXIgYmFkLgoKPiAKPiA+IFJldmlld2VkLWJ5OiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5 cmphbGFAbGludXguaW50ZWwuY29tPgo+ID4gCj4gPiBJIHN1cHBvc2Ugd2UgcmVhbGx5IHNob3Vs ZCBiZSBkb2luZyB0aGUgc2FtZSBmb3IgbW9zdCBvZiB0aGUgcmVzdCBvZgo+ID4gdmFyX3NjcmVl bmluZm8gYXMgd2UgZG9uJ3QgYWxsb3cgY2hhbmdpbmcgdGhlIHRpbWluZ3MvZXRjLiBlaXRoZXIu Cj4gCj4gVGltaW5nIGF0IGxlYXN0IGRvZW5zJ3QgaGF2ZSBhbiBpbW1lZGlhdGUgY29ycmVjdG5l c3MgaW1wYWN0IG9mIHVzZXJzcGFjZQo+IHJlbmRlcmluZyB0aGUgd3JvbmcgZm9ybWF0IDotKQo+ IAo+IFRoYW5rcyBmb3IgcmV2aWV3JnBhdGNoLCBhcHBsaWVkIHRvIGRybS1taXNjLWZpeGVzLgo+ IC1EYW5pZWwKPiAKPiA+IAo+ID4gPiArCj4gPiA+ICAvKioKPiA+ID4gICAqIGRybV9mYl9oZWxw ZXJfY2hlY2tfdmFyIC0gaW1wbGVtZW50YXRpb24gZm9yICZmYl9vcHMuZmJfY2hlY2tfdmFyCj4g PiA+ICAgKiBAdmFyOiBzY3JlZW5pbmZvIHRvIGNoZWNrCj4gPiA+IEBAIC0xNTkwLDcgKzE2MDks NiBAQCBpbnQgZHJtX2ZiX2hlbHBlcl9jaGVja192YXIoc3RydWN0IGZiX3Zhcl9zY3JlZW5pbmZv ICp2YXIsCj4gPiA+ICB7Cj4gPiA+ICAJc3RydWN0IGRybV9mYl9oZWxwZXIgKmZiX2hlbHBlciA9 IGluZm8tPnBhcjsKPiA+ID4gIAlzdHJ1Y3QgZHJtX2ZyYW1lYnVmZmVyICpmYiA9IGZiX2hlbHBl ci0+ZmI7Cj4gPiA+IC0JaW50IGRlcHRoOwo+ID4gPiAgCj4gPiA+ICAJaWYgKHZhci0+cGl4Y2xv Y2sgIT0gMCB8fCBpbl9kYmdfbWFzdGVyKCkpCj4gPiA+ICAJCXJldHVybiAtRUlOVkFMOwo+ID4g PiBAQCAtMTYxMCw3MiArMTYyOCwxNSBAQCBpbnQgZHJtX2ZiX2hlbHBlcl9jaGVja192YXIoc3Ry dWN0IGZiX3Zhcl9zY3JlZW5pbmZvICp2YXIsCj4gPiA+ICAJCXJldHVybiAtRUlOVkFMOwo+ID4g PiAgCX0KPiA+ID4gIAo+ID4gPiAtCXN3aXRjaCAodmFyLT5iaXRzX3Blcl9waXhlbCkgewo+ID4g PiAtCWNhc2UgMTY6Cj4gPiA+IC0JCWRlcHRoID0gKHZhci0+Z3JlZW4ubGVuZ3RoID09IDYpID8g MTYgOiAxNTsKPiA+ID4gLQkJYnJlYWs7Cj4gPiA+IC0JY2FzZSAzMjoKPiA+ID4gLQkJZGVwdGgg PSAodmFyLT50cmFuc3AubGVuZ3RoID4gMCkgPyAzMiA6IDI0Owo+ID4gPiAtCQlicmVhazsKPiA+ ID4gLQlkZWZhdWx0Ogo+ID4gPiAtCQlkZXB0aCA9IHZhci0+Yml0c19wZXJfcGl4ZWw7Cj4gPiA+ IC0JCWJyZWFrOwo+ID4gPiAtCX0KPiA+ID4gLQo+ID4gPiAtCXN3aXRjaCAoZGVwdGgpIHsKPiA+ ID4gLQljYXNlIDg6Cj4gPiA+IC0JCXZhci0+cmVkLm9mZnNldCA9IDA7Cj4gPiA+IC0JCXZhci0+ Z3JlZW4ub2Zmc2V0ID0gMDsKPiA+ID4gLQkJdmFyLT5ibHVlLm9mZnNldCA9IDA7Cj4gPiA+IC0J CXZhci0+cmVkLmxlbmd0aCA9IDg7Cj4gPiA+IC0JCXZhci0+Z3JlZW4ubGVuZ3RoID0gODsKPiA+ ID4gLQkJdmFyLT5ibHVlLmxlbmd0aCA9IDg7Cj4gPiA+IC0JCXZhci0+dHJhbnNwLmxlbmd0aCA9 IDA7Cj4gPiA+IC0JCXZhci0+dHJhbnNwLm9mZnNldCA9IDA7Cj4gPiA+IC0JCWJyZWFrOwo+ID4g PiAtCWNhc2UgMTU6Cj4gPiA+IC0JCXZhci0+cmVkLm9mZnNldCA9IDEwOwo+ID4gPiAtCQl2YXIt PmdyZWVuLm9mZnNldCA9IDU7Cj4gPiA+IC0JCXZhci0+Ymx1ZS5vZmZzZXQgPSAwOwo+ID4gPiAt CQl2YXItPnJlZC5sZW5ndGggPSA1Owo+ID4gPiAtCQl2YXItPmdyZWVuLmxlbmd0aCA9IDU7Cj4g PiA+IC0JCXZhci0+Ymx1ZS5sZW5ndGggPSA1Owo+ID4gPiAtCQl2YXItPnRyYW5zcC5sZW5ndGgg PSAxOwo+ID4gPiAtCQl2YXItPnRyYW5zcC5vZmZzZXQgPSAxNTsKPiA+ID4gLQkJYnJlYWs7Cj4g PiA+IC0JY2FzZSAxNjoKPiA+ID4gLQkJdmFyLT5yZWQub2Zmc2V0ID0gMTE7Cj4gPiA+IC0JCXZh ci0+Z3JlZW4ub2Zmc2V0ID0gNTsKPiA+ID4gLQkJdmFyLT5ibHVlLm9mZnNldCA9IDA7Cj4gPiA+ IC0JCXZhci0+cmVkLmxlbmd0aCA9IDU7Cj4gPiA+IC0JCXZhci0+Z3JlZW4ubGVuZ3RoID0gNjsK PiA+ID4gLQkJdmFyLT5ibHVlLmxlbmd0aCA9IDU7Cj4gPiA+IC0JCXZhci0+dHJhbnNwLmxlbmd0 aCA9IDA7Cj4gPiA+IC0JCXZhci0+dHJhbnNwLm9mZnNldCA9IDA7Cj4gPiA+IC0JCWJyZWFrOwo+ ID4gPiAtCWNhc2UgMjQ6Cj4gPiA+IC0JCXZhci0+cmVkLm9mZnNldCA9IDE2Owo+ID4gPiAtCQl2 YXItPmdyZWVuLm9mZnNldCA9IDg7Cj4gPiA+IC0JCXZhci0+Ymx1ZS5vZmZzZXQgPSAwOwo+ID4g PiAtCQl2YXItPnJlZC5sZW5ndGggPSA4Owo+ID4gPiAtCQl2YXItPmdyZWVuLmxlbmd0aCA9IDg7 Cj4gPiA+IC0JCXZhci0+Ymx1ZS5sZW5ndGggPSA4Owo+ID4gPiAtCQl2YXItPnRyYW5zcC5sZW5n dGggPSAwOwo+ID4gPiAtCQl2YXItPnRyYW5zcC5vZmZzZXQgPSAwOwo+ID4gPiAtCQlicmVhazsK PiA+ID4gLQljYXNlIDMyOgo+ID4gPiAtCQl2YXItPnJlZC5vZmZzZXQgPSAxNjsKPiA+ID4gLQkJ dmFyLT5ncmVlbi5vZmZzZXQgPSA4Owo+ID4gPiAtCQl2YXItPmJsdWUub2Zmc2V0ID0gMDsKPiA+ ID4gLQkJdmFyLT5yZWQubGVuZ3RoID0gODsKPiA+ID4gLQkJdmFyLT5ncmVlbi5sZW5ndGggPSA4 Owo+ID4gPiAtCQl2YXItPmJsdWUubGVuZ3RoID0gODsKPiA+ID4gLQkJdmFyLT50cmFuc3AubGVu Z3RoID0gODsKPiA+ID4gLQkJdmFyLT50cmFuc3Aub2Zmc2V0ID0gMjQ7Cj4gPiA+IC0JCWJyZWFr Owo+ID4gPiAtCWRlZmF1bHQ6Cj4gPiA+ICsJLyoKPiA+ID4gKwkgKiBkcm0gZmJkZXYgZW11bGF0 aW9uIGRvZXNuJ3Qgc3VwcG9ydCBjaGFuZ2luZyB0aGUgcGl4ZWwgZm9ybWF0IGF0IGFsbCwKPiA+ ID4gKwkgKiBzbyByZWplY3QgYWxsIHBpeGVsIGZvcm1hdCBjaGFuZ2luZyByZXF1ZXN0cy4KPiA+ ID4gKwkgKi8KPiA+ID4gKwlpZiAoIWRybV9mYl9waXhlbF9mb3JtYXRfZXF1YWwodmFyLCAmaW5m by0+dmFyKSkgewo+ID4gPiArCQlEUk1fREVCVUcoImZiZGV2IGVtdWxhdGlvbiBkb2Vzbid0IHN1 cHBvcnQgY2hhbmdpbmcgdGhlIHBpeGVsIGZvcm1hdFxuIik7Cj4gPiA+ICAJCXJldHVybiAtRUlO VkFMOwo+ID4gPiAgCX0KPiA+ID4gKwo+ID4gPiAgCXJldHVybiAwOwo+ID4gPiAgfQo+ID4gPiAg RVhQT1JUX1NZTUJPTChkcm1fZmJfaGVscGVyX2NoZWNrX3Zhcik7Cj4gPiA+IC0tIAo+ID4gPiAy LjE0LjQKPiA+ID4gCj4gPiA+IF9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCj4gPiA+IGRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKPiA+ID4gZHJpLWRldmVsQGxp c3RzLmZyZWVkZXNrdG9wLm9yZwo+ID4gPiBodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9t YWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo+ID4gCj4gPiAtLSAKPiA+IFZpbGxlIFN5cmrDpGzD pAo+ID4gSW50ZWwKPiA+IF9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCj4gPiBkcmktZGV2ZWwgbWFpbGluZyBsaXN0Cj4gPiBkcmktZGV2ZWxAbGlzdHMuZnJl ZWRlc2t0b3Aub3JnCj4gPiBodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2RyaS1kZXZlbAo+IAo+IC0tIAo+IERhbmllbCBWZXR0ZXIKPiBTb2Z0d2FyZSBFbmdp bmVlciwgSW50ZWwgQ29ycG9yYXRpb24KPiBodHRwOi8vYmxvZy5mZndsbC5jaAoKLS0gClZpbGxl IFN5cmrDpGzDpApJbnRlbApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14D05C64EBC for ; Thu, 4 Oct 2018 11:00:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CEC4A2064E for ; Thu, 4 Oct 2018 11:00:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CEC4A2064E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727385AbeJDRxe (ORCPT ); Thu, 4 Oct 2018 13:53:34 -0400 Received: from mga04.intel.com ([192.55.52.120]:58703 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727131AbeJDRxd (ORCPT ); Thu, 4 Oct 2018 13:53:33 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Oct 2018 04:00:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,338,1534834800"; d="scan'208";a="78646832" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by orsmga008.jf.intel.com with SMTP; 04 Oct 2018 04:00:44 -0700 Received: by stinkbox (sSMTP sendmail emulation); Thu, 04 Oct 2018 14:00:43 +0300 Date: Thu, 4 Oct 2018 14:00:43 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Eugeniy Paltsev , David Airlie , Alexey Brodkin , linux-kernel@vger.kernel.org, stable@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-snps-arc@lists.infradead.org, Sean Paul Subject: Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests Message-ID: <20181004110043.GP9144@intel.com> References: <20181003164538.5534-1-Eugeniy.Paltsev@synopsys.com> <20181004103422.GL9144@intel.com> <20181004104938.GJ31561@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181004104938.GJ31561@phenom.ffwll.local> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > --- > > > 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ä > > > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:58703 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727131AbeJDRxd (ORCPT ); Thu, 4 Oct 2018 13:53:33 -0400 Date: Thu, 4 Oct 2018 14:00:43 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Eugeniy Paltsev , David Airlie , Alexey Brodkin , linux-kernel@vger.kernel.org, stable@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-snps-arc@lists.infradead.org, Sean Paul Subject: Re: [PATCH v2] drm: fb-helper: Reject all pixel format changing requests Message-ID: <20181004110043.GP9144@intel.com> References: <20181003164538.5534-1-Eugeniy.Paltsev@synopsys.com> <20181004103422.GL9144@intel.com> <20181004104938.GJ31561@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181004104938.GJ31561@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org List-ID: 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 > > > --- > > > 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� > > > > 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