Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 1 Sep 2020 12:19:23 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        Eric Anholt <eric@anholt.net>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        linux-rpi-kernel@lists.infradead.org,
        bcm-kernel-feedback-list@broadcom.com,
        linux-arm-kernel@lists.infradead.org,
        LKML <linux-kernel@vger.kernel.org>,
        Tim Gover <tim.gover@raspberrypi.com>,
        Phil Elwell <phil@raspberrypi.com>
Subject: Re: [PATCH v4 77/78] drm/vc4: drv: Support BCM2711
Message-ID: <20200901101923.7e744qwtuvmnbtmn@gilmour.lan>
References: <cover.7a1aa1784976093af26cb31fd283cf5b3ed568bb.1594230107.git-series.maxime@cerno.tech>
 <90c53367b289e9c0f9eeb97794a8d3b08d813884.1594230107.git-series.maxime@cerno.tech>
 <CAPY8ntB3eeqqz0hPSfHiJpKs9sJpU5uN5GzLsqmhtxrD0i6TJg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="dozkezvx4o5iboho"
Content-Disposition: inline
In-Reply-To: <CAPY8ntB3eeqqz0hPSfHiJpKs9sJpU5uN5GzLsqmhtxrD0i6TJg@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--dozkezvx4o5iboho
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Dave,

On Tue, Jul 28, 2020 at 04:30:16PM +0100, Dave Stevenson wrote:
> > @@ -681,10 +684,14 @@ int vc4_kms_load(struct drm_device *dev)
> >         struct vc4_load_tracker_state *load_state;
> >         int ret;
> >
> > -       /* Start with the load tracker enabled. Can be disabled through=
 the
> > -        * debugfs load_tracker file.
> > -        */
> > -       vc4->load_tracker_enabled =3D true;
> > +       if (!of_device_is_compatible(dev->dev->of_node, "brcm,bcm2711-v=
c5")) {
>=20
> Is it better to look up the compatible string, or pass something via
> the .data element of the of_device_id table? Probably down to personal
> preference?

It's pretty much equivalent, so I'm not sure one is arguably better than
the other. However, checking for the compatible can be pretty cumbersome
when you have to do it repeatedly (like we do in the HDMI controller),
and when you don't it a lot, having a structure associated to the
compatible is also fairly cumbersome.

> > +               vc4->load_tracker_available =3D true;
> > +
> > +               /* Start with the load tracker enabled. Can be
> > +                * disabled through the debugfs load_tracker file.
> > +                */
> > +               vc4->load_tracker_enabled =3D true;
> > +       }
> >
> >         sema_init(&vc4->async_modeset, 1);
> >
> > @@ -698,8 +705,14 @@ int vc4_kms_load(struct drm_device *dev)
> >                 return ret;
> >         }
> >
> > -       dev->mode_config.max_width =3D 2048;
> > -       dev->mode_config.max_height =3D 2048;
> > +       if (of_device_is_compatible(dev->dev->of_node, "brcm,bcm2711-vc=
5")) {
>=20
> We're making the same of_device_is_compatible call twice within
> vc4_kms_load. Set a flag based on it and check that instead?

Good idea, thanks!
Maxime

--dozkezvx4o5iboho
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX04gKwAKCRDj7w1vZxhR
xdjRAP9NRCUkLEG71G/PlhSmiqu8PAfDEFoqOIOabvM3/kj1GAEAxUSCGhv9XmUs
ZhR7LLKC9pnKW5c295ozmuSloS08AQg=
=s4r0
-----END PGP SIGNATURE-----

--dozkezvx4o5iboho--