From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Make sure at least one plane supports the fb format Date: Mon, 05 Mar 2018 14:56:59 -0800 Message-ID: <87woyq6shw.fsf@anholt.net> References: <20180305144919.2881-1-ville.syrjala@linux.intel.com> <87y3j6utm3.fsf@anholt.net> <20180305211500.GY5453@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1126222211==" Return-path: In-Reply-To: <20180305211500.GY5453@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1126222211== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ville Syrj=C3=A4l=C3=A4 writes: > On Mon, Mar 05, 2018 at 12:59:00PM -0800, Eric Anholt wrote: >> Ville Syrjala writes: >>=20 >> > From: Ville Syrj=C3=A4l=C3=A4 >> > >> > To make life easier for drivers, let's have the core check that the >> > requested pixel format is supported by at least one plane when creating >> > a new framebuffer. >> > >> > Signed-off-by: Ville Syrj=C3=A4l=C3=A4 >> > --- >> > drivers/gpu/drm/drm_framebuffer.c | 26 ++++++++++++++++++++++++++ >> > 1 file changed, 26 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_f= ramebuffer.c >> > index c0530a1af5e3..155b21e579c4 100644 >> > --- a/drivers/gpu/drm/drm_framebuffer.c >> > +++ b/drivers/gpu/drm/drm_framebuffer.c >> > @@ -152,6 +152,23 @@ static int fb_plane_height(int height, >> > return DIV_ROUND_UP(height, format->vsub); >> > } >> >=20=20 >> > +static bool planes_have_format(struct drm_device *dev, u32 format) >> > +{ >> > + struct drm_plane *plane; >> > + >> > + /* TODO: maybe maintain a device level format list? */ >> > + drm_for_each_plane(plane, dev) { >> > + int i; >> > + >> > + for (i =3D 0; i < plane->format_count; i++) { >> > + if (plane->format_types[i] =3D=3D format) >> > + return true; >> > + } >> > + } >> > + >> > + return false; >> > +} >> > + >> > static int framebuffer_check(struct drm_device *dev, >> > const struct drm_mode_fb_cmd2 *r) >> > { >> > @@ -168,6 +185,15 @@ static int framebuffer_check(struct drm_device *d= ev, >> > return -EINVAL; >> > } >> >=20=20 >> > + if (!planes_have_format(dev, r->pixel_format)) { >> > + struct drm_format_name_buf format_name; >> > + >> > + DRM_DEBUG_KMS("unsupported framebuffer format %s\n", >> > + drm_get_format_name(r->pixel_format, >> > + &format_name)); >> > + return -EINVAL; >> > + } >> > + >>=20 >> Won't this break KMS on things like the radeon driver, which doesn't do >> planes? Maybe check if any universal planes have been registered and >> only do the check in that case? > > Hmm. I thought we add the implicit planes always. Apparently > drm_crtc_init() adds a primary with X/ARGB8888, but no more. So > this would break all other formats, which is probably a bit too > aggressive. > > I guess I could just skip the check in case any plane has > plane->format_default set. That should be indicating that the driver > doesn't do planes fully. Oh, why exactly is amggpu setting that flag? > Harry? > >>=20 >> Also, "any_planes_have_format()" might be slightly more descriptive. > > Or any_plane_has_format()? Is that more englishy? :) I like it. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlqdyzsACgkQtdYpNtH8 nuizghAArkNegGVM/+v7LAEFCjl4o69ZrtElXbq3KQR2Y5ID4g8hN2jNYNrsGlIu ulvcDR/EPHbFoFkzveMwchelUofn4wBTg6/yzeJ6yTltqd+y51hmx7Iq7I5wxozC H82G9WQLb7LJp5YTKZzE2sXOy8Up2OM0s4vzAz3eljrCGsHsOlUnoLnxl8Taqx28 yZGWgSTMyBPZwzPoYJcT8mioFQlQDNePn+XuTW/iRuH91AwF0TrgvJaUt6MaIzmv wzPU44DBuGx8XQ6AdhXOzxwVf9rxIMjDbOlrRGEKF/ZO+fHRm2reqJgmQv3iNIin jagH4janiJqF16FkklAbIMkjPBEFleUCdEDkOjz+QlGDAUqcXoyjHD33falwzARj Q4grN0xS+nL3LTqZgh5jo9vXxpFIBlNsWa8aqdLCna5lKRy9bmncaZHmz8zv7jUj lMJTb50h/R71202+P/NlBpCJef6XmxpJ7H9vk4zd553WR14CzAJomogJNQ/0qkbv IRMxMSOcW4tIns37e3uMNkzmM9m0j+kWQ0yaR3Kk9X3xJwaokpwkCRvHDm3rAnaq NhGqYGC2dXIQfdvnV5rzAgR3zJF3K/ze3e7QPcCuaOBUl5AgrDDlCfphAn8qUWL6 +9mUuzIPkeqhWtSbcNyUIrpuolbpT0FFLKf4TdSkyRV4h4xfq8I= =cu5G -----END PGP SIGNATURE----- --=-=-=-- --===============1126222211== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1126222211==--