Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753454AbcKUHk5 (ORCPT ); Mon, 21 Nov 2016 02:40:57 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:49515 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752180AbcKUHk4 (ORCPT ); Mon, 21 Nov 2016 02:40:56 -0500 Date: Mon, 21 Nov 2016 08:40:53 +0100 From: Maxime Ripard To: Sean Paul Cc: Daniel Vetter , David Airlie , dri-devel , Laurent Pinchart , Boris Brezillon , Thomas Petazzoni , Linux ARM Kernel , Hans de Goede , Chen-Yu Tsai , Linux Kernel Mailing List Subject: Re: [PATCH 2/5] drm/modes: Support modes names on the command line Message-ID: <20161121074053.jguhh3kpgm7nyf2b@lukather> References: <853193fdf9ec1af94056d9fa2c276079c779aaf4.1476779323.git-series.maxime.ripard@free-electrons.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nhgf7ewut7pwlfhl" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8172 Lines: 206 --nhgf7ewut7pwlfhl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Sean, On Wed, Nov 16, 2016 at 12:21:42PM -0500, Sean Paul wrote: > On Tue, Oct 18, 2016 at 4:29 AM, Maxime Ripard > wrote: > > The drm subsystem also uses the video=3D kernel parameter, and in the > > documentation refers to the fbdev documentation for that parameter. > > > > However, that documentation also says that instead of giving the mode u= sing > > its resolution we can also give a name. However, DRM doesn't handle that > > case at the moment. Even though in most case it shouldn't make any > > difference, it might be useful for analog modes, where different standa= rds > > might have the same resolution, but still have a few different paramete= rs > > that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example). > > > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/drm_connector.c | 3 +- > > drivers/gpu/drm/drm_fb_helper.c | 4 +++- > > drivers/gpu/drm/drm_modes.c | 49 +++++++++++++++++++++++----------- > > include/drm/drm_connector.h | 1 +- > > 4 files changed, 41 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_conn= ector.c > > index 2db7fb510b6c..27a8a511257c 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -147,8 +147,9 @@ static void drm_connector_get_cmdline_mode(struct d= rm_connector *connector) > > connector->force =3D mode->force; > > } > > > > - DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n= ", > > + DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%= s\n", > > connector->name, > > + mode->name ? mode->name : "", > > mode->xres, mode->yres, > > mode->refresh_specified ? mode->refresh : 60, > > mode->rb ? " reduced blanking" : "", > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_h= elper.c > > index 03414bde1f15..20a68305fb45 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -1748,6 +1748,10 @@ struct drm_display_mode *drm_pick_cmdline_mode(s= truct drm_fb_helper_connector *f > > prefer_non_interlace =3D !cmdline_mode->interlace; > > again: > > list_for_each_entry(mode, &fb_helper_conn->connector->modes, he= ad) { > > + /* Check (optional) mode name first */ > > + if (!strcmp(mode->name, cmdline_mode->name)) > > + return mode; > > + > > /* check width/height */ > > if (mode->hdisplay !=3D cmdline_mode->xres || > > mode->vdisplay !=3D cmdline_mode->yres) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index 7d5bdca276f2..fdbf541a5978 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -1413,7 +1413,7 @@ bool drm_mode_parse_command_line_for_connector(co= nst char *mode_option, > > struct drm_cmdline_mode = *mode) > > { > > const char *name; > > - bool parse_extras =3D false; > > + bool named_mode =3D false, parse_extras =3D false; > > unsigned int bpp_off =3D 0, refresh_off =3D 0; > > unsigned int mode_end =3D 0; > > char *bpp_ptr =3D NULL, *refresh_ptr =3D NULL, *extra_ptr =3D N= ULL; > > @@ -1432,8 +1432,14 @@ bool drm_mode_parse_command_line_for_connector(c= onst char *mode_option, > > > > name =3D mode_option; > > > > + /* > > + * If the first character is not a digit, then it means that > > + * we have a named mode. > > + */ > > if (!isdigit(name[0])) > > - return false; > > + named_mode =3D true; > > + else > > + named_mode =3D false; >=20 > named_mode =3D isalpha(name[0]); might be more succinct (and covers > special characters). >=20 > > > > /* Try to locate the bpp and refresh specifiers, if any */ > > bpp_ptr =3D strchr(name, '-'); > > @@ -1460,12 +1466,16 @@ bool drm_mode_parse_command_line_for_connector(= const char *mode_option, > > parse_extras =3D true; > > } > > > > - ret =3D drm_mode_parse_cmdline_res_mode(name, mode_end, > > - parse_extras, > > - connector, > > - mode); > > - if (ret) > > - return false; > > + if (named_mode) { > > + strncpy(mode->name, name, mode_end); > > + } else { > > + ret =3D drm_mode_parse_cmdline_res_mode(name, mode_end, > > + parse_extras, > > + connector, > > + mode); > > + if (ret) > > + return false; > > + } > > mode->specified =3D true; > > > > if (bpp_ptr) { > > @@ -1493,14 +1503,23 @@ bool drm_mode_parse_command_line_for_connector(= const char *mode_option, > > extra_ptr =3D refresh_end_ptr; > > > > if (extra_ptr) { > > - int remaining =3D strlen(name) - (extra_ptr - name); > > + if (!named_mode) { > > + int len =3D strlen(name) - (extra_ptr - name); > > > > - /* > > - * We still have characters to process, while > > - * we shouldn't have any > > - */ > > - if (remaining > 0) > > - return false; > > + ret =3D drm_mode_parse_cmdline_extra(extra_ptr,= len, > > + connector, m= ode); > > + if (ret) > > + return false; > > + } else { > > + int remaining =3D strlen(name) - (extra_ptr - n= ame); > > + > > + /* > > + * We still have characters to process, while > > + * we shouldn't have any > > + */ > > + if (remaining > 0) > > + return false; >=20 > Correct me if I'm wrong, but this shouldn't ever happen. AFAICT, the > only way it could would be if we parsed bpp or refresh in the named > mode (since those are the only cases where we don't copy the whole > string over). Shouldn't that be invalid anyways? It's supposed to be supported by the video string. Documentation/fb/modedb.txt defines: Valid mode specifiers (mode_option argument): x[M][R][-][@][i][m][eDd] [-][@] So we can't just copy the string over, and we need to parse it :/ Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --nhgf7ewut7pwlfhl Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYMqUEAAoJEBx+YmzsjxAgV80QAI92H0gbsSfIJ3WReGrG/TId koTuVU0MyTD8TJmEzuo2XyTDKnHQLCVequ7zRg/zRcZj5gYwmOHRMjPtz+O4BU1s 2zoWnB6jYJgjMxoxCD7TKb9OtFdeKEg1MiYYR6Ve4c1f+eZII2Zk023ffNMgs0a8 KIL2nGAFyjLoIMZVf+lO4TWQ/sMQgsHglRqjR1J10py2CMhnwevQaWVGLpXl45mK PpSJW6kq3ZOOx5my9kUyQmJPHpr4Af8ibiLzAnXjtN1m9DN/pOcJDn6LTlgjz6JD mDjNbeQcRfDcaG6XxW42Rn2SWf0kRiUXdv99dI+D2bipYcPseV9YoXTezpIegZai 6x5KZ+rpfRmFm7UJYipSQFT9yfOTpWtMl/ZR9Smzp2ChMd7FZgBkVxclL9DEXaQQ bGF7b2O7cHgAx4kT7YTWLYUxqjwCRFEaZuFjIymmGQ+blSsyK2MxZLC09EXoDjgP LZVW+QHVlwzJCHdGtbG+6/lmz9GfOztv2XEZ9SfVRDb0XvmlpAOx/TUzVU2xx7w6 x5PmAN7bwQUSLokg7DVy/4DAVbohxktqAcgLQSV3cBUx2Tmy5npk9kgpkAoP7D3u oXm1FpRVUSnWLf+Seec7OcB1Ndq1qZ7z0Mbi6RHT9RQUSWl5ihHm1j4sJk5+2Y97 phEKYO0heDTgB1eai1Jn =Usur -----END PGP SIGNATURE----- --nhgf7ewut7pwlfhl--