lkml.org 
[lkml]   [2022]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 09/35] drm/modes: Move named modes parsing to a separate function
On Fri, Aug 12, 2022 at 03:27:17PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> On Fri, Jul 29, 2022 at 6:36 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > The current construction of the named mode parsing doesn't allow to extend
> > it easily. Let's move it to a separate function so we can add more
> > parameters and modes.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>
> Thanks for your patch, which looks similar to my "[PATCH v2 2/5]
> drm/modes: Extract drm_mode_parse_cmdline_named_mode()"
> (https://lore.kernel.org/dri-devel/1371554419ae63cb54c2a377db0c1016fcf200bb.1657788997.git.geert@linux-m68k.org
> ;-)

Indeed, I forgot about that one, sorry :/

I think I'd still prefer to have the check for refresh + named mode
outside of the function, since I see them as an "integration" issue, not
a parsing one.

It's not the named mode parsing that fails, but the fact that we both
have a valid refresh and a valid named mode.

>
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1773,6 +1773,28 @@ static const char * const drm_named_modes_whitelist[] = {
> > "PAL",
> > };
> >
> > +static bool drm_mode_parse_cmdline_named_mode(const char *name,
> > + unsigned int name_end,
> > + struct drm_cmdline_mode *cmdline_mode)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> > + int ret;
> > +
> > + ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> > + if (ret != name_end)
> > + continue;
> > +
> > + strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]);
> > + cmdline_mode->specified = true;
> > +
> > + return true;
> > + }
> > +
> > + return false;
>
> What's the point in returning a value, if it is never checked?
> Just make this function return void?

Yeah, it's something I went back and forth to between the dev, and it's
an artifact.

I'll drop that patch, take your version and move the refresh check to
drm_mode_parse_command_line_for_connector if that's alright for you?

Maxime
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-08-16 15:47    [W:1.266 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site