Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i8771445c:Fastmail
Date:   Wed, 9 Nov 2022 16:41:33 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>
Cc:     Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Ben Skeggs <bskeggs@redhat.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Samuel Holland <samuel@sholland.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Emma Anholt <emma@anholt.net>,
        Karol Herbst <kherbst@redhat.com>,
        Daniel Vetter <daniel@ffwll.ch>, Chen-Yu Tsai <wens@csie.org>,
        Lyude Paul <lyude@redhat.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@linux.ie>,
        Phil Elwell <phil@raspberrypi.com>,
        Hans de Goede <hdegoede@redhat.com>,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        linux-arm-kernel@lists.infradead.org,
        Dom Cobley <dom@raspberrypi.com>,
        Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>,
        dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
        nouveau@lists.freedesktop.org
Subject: Re: [PATCH v7 16/23] drm/probe-helper: Provide a TV get_modes helper
Message-ID: <20221109154133.koqhn6upwz6jd2oe@houat>
References: <20220728-rpi-analog-tv-properties-v7-0-7072a478c6b3@cerno.tech>
 <20220728-rpi-analog-tv-properties-v7-16-7072a478c6b3@cerno.tech>
 <a9c0380a-f538-1a19-fd27-983eea42b1b2@tronnes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <a9c0380a-f538-1a19-fd27-983eea42b1b2@tronnes.org>
Precedence: bulk

Hi Noralf,

On Mon, Nov 07, 2022 at 07:11:27PM +0100, Noralf Tr=F8nnes wrote:
>=20
>=20
> Den 07.11.2022 15.16, skrev Maxime Ripard:
> > From: Noralf Tr=F8nnes <noralf@tronnes.org>
> >=20
> > Most of the TV connectors will need a similar get_modes implementation
> > that will, depending on the drivers' capabilities, register the 480i and
> > 576i modes.
> >=20
> > That implementation will also need to set the preferred flag and order
> > the modes based on the driver and users preferrence.
> >=20
> > This is especially important to guarantee that a userspace stack such as
> > Xorg can start and pick up the preferred mode while maintaining a
> > working output.
> >=20
> > Signed-off-by: Noralf Tr=F8nnes <noralf@tronnes.org>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >=20
> > ---
> > Changes in v7:
> > - Used Noralf's implementation
> >=20
> > Changes in v6:
> > - New patch
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 97 ++++++++++++++++++++++++++++++=
++++++++
> >  include/drm/drm_probe_helper.h     |  1 +
> >  2 files changed, 98 insertions(+)
> >=20
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_p=
robe_helper.c
> > index 2fc21df709bc..edb2e4c4530a 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -1147,3 +1147,100 @@ int drm_connector_helper_get_modes(struct drm_c=
onnector *connector)
> >  	return count;
> >  }
> >  EXPORT_SYMBOL(drm_connector_helper_get_modes);
> > +
> > +static bool tv_mode_supported(struct drm_connector *connector,
> > +			      enum drm_connector_tv_mode mode)
> > +{
> > +	struct drm_device *dev =3D connector->dev;
> > +	struct drm_property *property =3D dev->mode_config.tv_mode_property;
> > +
> > +	unsigned int i;
> > +
> > +	for (i =3D 0; i < property->num_values; i++)
> > +		if (property->values[i] =3D=3D mode)
> > +			return true;
> > +
> > +	return false;
> > +}
>=20
> This function is not used in the new implementation.
>
> I hope you have tested this patch since I didn't even compile test my
> implementation (probably should have said so...)

You nailed it ;)

I had tested it (but missed the warning), and added unit tests to make
sure it was behaving properly, and it did. I'll send the unit tests in
my next version.

Thanks
Maxime