From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Date: Fri, 02 Feb 2018 14:03:43 +0000 Subject: Re: [PATCH] drm/fb-helper: Scale back depth to supported maximum Message-Id: <20180202140343.GX5453@intel.com> List-Id: References: <20180201130446.6165-1-linus.walleij@linaro.org> <20180201131907.GO5453@intel.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Linus Walleij Cc: linux-fbdev@vger.kernel.org, David Lechner , Bartlomiej Zolnierkiewicz , "open list:DRM PANEL DRIVERS" , Dave Airlie , Linux ARM On Fri, Feb 02, 2018 at 02:56:30PM +0100, Linus Walleij wrote: > On Thu, Feb 1, 2018 at 2:19 PM, Ville Syrj=E4l=E4 > wrote: > > [Me] > >> + /* > >> + * If we run into a situation where, for example, the primary pl= ane > >> + * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, = depth > >> + * 16) we need to scale down the depth of the sizes we request. > >> + */ > >> + drm_for_each_plane(plane, fb_helper->dev) { > >> + /* Only check the primary plane */ > >> + if (plane->type !=3D DRM_PLANE_TYPE_PRIMARY) > >> + continue; > > > > I think this should look at crtc->primary for each of the crtcs managed > > by the fb_helper. > > > > Also this probably shouldn't look at YUV formats at all? >=20 > I guess I can look into doing it this way, sorry for not knowing how to > properly inspect DRM objects, I'm lost sometimes... >=20 > > I do wonder if instead we should just have the driver specify the > > pixel format explicitly instead of trying to guess based on bpp? >=20 > That makes a lot more sense to me actually. It would > give a better sense of control so the driver feel it knows > what is actually going on. >=20 > So I would just update > drm_fb_cma_fbdev_init() and drm_fb_helper_initial_config() > to pass a reasonable pixel format instead and refactor all the > way down? Yeah, something along those lines would seem like the better approach to me. But it's been a while since I've looked at this code so I might be totally wrong :) >=20 > It does hit a lot of code on the way, but if everyone thinks this > is a good idea I can very well take a stab at it. >=20 > Yours, > Linus Walleij --=20 Ville Syrj=E4l=E4 Intel OTC 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: Fri, 2 Feb 2018 16:03:43 +0200 Subject: [PATCH] drm/fb-helper: Scale back depth to supported maximum In-Reply-To: References: <20180201130446.6165-1-linus.walleij@linaro.org> <20180201131907.GO5453@intel.com> Message-ID: <20180202140343.GX5453@intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Feb 02, 2018 at 02:56:30PM +0100, Linus Walleij wrote: > On Thu, Feb 1, 2018 at 2:19 PM, Ville Syrj?l? > wrote: > > [Me] > >> + /* > >> + * If we run into a situation where, for example, the primary plane > >> + * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth > >> + * 16) we need to scale down the depth of the sizes we request. > >> + */ > >> + drm_for_each_plane(plane, fb_helper->dev) { > >> + /* Only check the primary plane */ > >> + if (plane->type != DRM_PLANE_TYPE_PRIMARY) > >> + continue; > > > > I think this should look at crtc->primary for each of the crtcs managed > > by the fb_helper. > > > > Also this probably shouldn't look at YUV formats at all? > > I guess I can look into doing it this way, sorry for not knowing how to > properly inspect DRM objects, I'm lost sometimes... > > > I do wonder if instead we should just have the driver specify the > > pixel format explicitly instead of trying to guess based on bpp? > > That makes a lot more sense to me actually. It would > give a better sense of control so the driver feel it knows > what is actually going on. > > So I would just update > drm_fb_cma_fbdev_init() and drm_fb_helper_initial_config() > to pass a reasonable pixel format instead and refactor all the > way down? Yeah, something along those lines would seem like the better approach to me. But it's been a while since I've looked at this code so I might be totally wrong :) > > It does hit a lot of code on the way, but if everyone thinks this > is a good idea I can very well take a stab at it. > > Yours, > Linus Walleij -- Ville Syrj?l? Intel OTC From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/fb-helper: Scale back depth to supported maximum Date: Fri, 2 Feb 2018 16:03:43 +0200 Message-ID: <20180202140343.GX5453@intel.com> References: <20180201130446.6165-1-linus.walleij@linaro.org> <20180201131907.GO5453@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id E4BDA6E18F for ; Fri, 2 Feb 2018 14:03:48 +0000 (UTC) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Linus Walleij Cc: linux-fbdev@vger.kernel.org, David Lechner , Bartlomiej Zolnierkiewicz , "open list:DRM PANEL DRIVERS" , Dave Airlie , Linux ARM List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBGZWIgMDIsIDIwMTggYXQgMDI6NTY6MzBQTSArMDEwMCwgTGludXMgV2FsbGVpaiB3 cm90ZToKPiBPbiBUaHUsIEZlYiAxLCAyMDE4IGF0IDI6MTkgUE0sIFZpbGxlIFN5cmrDpGzDpAo+ IDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4gd3JvdGU6Cj4gPiBbTWVdCj4gPj4gKyAg ICAgLyoKPiA+PiArICAgICAgKiBJZiB3ZSBydW4gaW50byBhIHNpdHVhdGlvbiB3aGVyZSwgZm9y IGV4YW1wbGUsIHRoZSBwcmltYXJ5IHBsYW5lCj4gPj4gKyAgICAgICogc3VwcG9ydHMgUkdCQTU1 NTEgKDE2IGJwcCwgZGVwdGggMTUpIGJ1dCBub3QgUkdCNTY1ICgxNiBicHAsIGRlcHRoCj4gPj4g KyAgICAgICogMTYpIHdlIG5lZWQgdG8gc2NhbGUgZG93biB0aGUgZGVwdGggb2YgdGhlIHNpemVz IHdlIHJlcXVlc3QuCj4gPj4gKyAgICAgICovCj4gPj4gKyAgICAgZHJtX2Zvcl9lYWNoX3BsYW5l KHBsYW5lLCBmYl9oZWxwZXItPmRldikgewo+ID4+ICsgICAgICAgICAgICAgLyogT25seSBjaGVj ayB0aGUgcHJpbWFyeSBwbGFuZSAqLwo+ID4+ICsgICAgICAgICAgICAgaWYgKHBsYW5lLT50eXBl ICE9IERSTV9QTEFORV9UWVBFX1BSSU1BUlkpCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIGNv bnRpbnVlOwo+ID4KPiA+IEkgdGhpbmsgdGhpcyBzaG91bGQgbG9vayBhdCBjcnRjLT5wcmltYXJ5 IGZvciBlYWNoIG9mIHRoZSBjcnRjcyBtYW5hZ2VkCj4gPiBieSB0aGUgZmJfaGVscGVyLgo+ID4K PiA+IEFsc28gdGhpcyBwcm9iYWJseSBzaG91bGRuJ3QgbG9vayBhdCBZVVYgZm9ybWF0cyBhdCBh bGw/Cj4gCj4gSSBndWVzcyBJIGNhbiBsb29rIGludG8gZG9pbmcgaXQgdGhpcyB3YXksIHNvcnJ5 IGZvciBub3Qga25vd2luZyBob3cgdG8KPiBwcm9wZXJseSBpbnNwZWN0IERSTSBvYmplY3RzLCBJ J20gbG9zdCBzb21ldGltZXMuLi4KPiAKPiA+IEkgZG8gd29uZGVyIGlmIGluc3RlYWQgd2Ugc2hv dWxkIGp1c3QgaGF2ZSB0aGUgZHJpdmVyIHNwZWNpZnkgdGhlCj4gPiBwaXhlbCBmb3JtYXQgZXhw bGljaXRseSBpbnN0ZWFkIG9mIHRyeWluZyB0byBndWVzcyBiYXNlZCBvbiBicHA/Cj4gCj4gVGhh dCBtYWtlcyBhIGxvdCBtb3JlIHNlbnNlIHRvIG1lIGFjdHVhbGx5LiBJdCB3b3VsZAo+IGdpdmUg YSBiZXR0ZXIgc2Vuc2Ugb2YgY29udHJvbCBzbyB0aGUgZHJpdmVyIGZlZWwgaXQga25vd3MKPiB3 aGF0IGlzIGFjdHVhbGx5IGdvaW5nIG9uLgo+IAo+IFNvIEkgd291bGQganVzdCB1cGRhdGUKPiBk cm1fZmJfY21hX2ZiZGV2X2luaXQoKSBhbmQgZHJtX2ZiX2hlbHBlcl9pbml0aWFsX2NvbmZpZygp Cj4gdG8gcGFzcyBhIHJlYXNvbmFibGUgcGl4ZWwgZm9ybWF0IGluc3RlYWQgYW5kIHJlZmFjdG9y IGFsbCB0aGUKPiB3YXkgZG93bj8KClllYWgsIHNvbWV0aGluZyBhbG9uZyB0aG9zZSBsaW5lcyB3 b3VsZCBzZWVtIGxpa2UgdGhlIGJldHRlciBhcHByb2FjaAp0byBtZS4gQnV0IGl0J3MgYmVlbiBh IHdoaWxlIHNpbmNlIEkndmUgbG9va2VkIGF0IHRoaXMgY29kZSBzbyBJIG1pZ2h0CmJlIHRvdGFs bHkgd3JvbmcgOikKCj4gCj4gSXQgZG9lcyBoaXQgYSBsb3Qgb2YgY29kZSBvbiB0aGUgd2F5LCBi dXQgaWYgZXZlcnlvbmUgdGhpbmtzIHRoaXMKPiBpcyBhIGdvb2QgaWRlYSBJIGNhbiB2ZXJ5IHdl bGwgdGFrZSBhIHN0YWIgYXQgaXQuCj4gCj4gWW91cnMsCj4gTGludXMgV2FsbGVpagoKLS0gClZp bGxlIFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJl ZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGlu Zm8vZHJpLWRldmVsCg==