linux-kernel - Re: [PATCH v4 77/78] drm/vc4: drv: Support BCM2711

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200901101923.7e744qwtuvmnbtmn@gilmour.lan>
Date:   Tue, 1 Sep 2020 12:19:23 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Eric Anholt <eric@...olt.net>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        linux-rpi-kernel@...ts.infradead.org,
        bcm-kernel-feedback-list@...adcom.com,
        linux-arm-kernel@...ts.infradead.org,
        LKML <linux-kernel@...r.kernel.org>,
        Tim Gover <tim.gover@...pberrypi.com>,
        Phil Elwell <phil@...pberrypi.com>
Subject: Re: [PATCH v4 77/78] drm/vc4: drv: Support BCM2711

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 = true;
> > +       if (!of_device_is_compatible(dev->dev->of_node, "brcm,bcm2711-vc5")) {
> 
> 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 = true;
> > +
> > +               /* Start with the load tracker enabled. Can be
> > +                * disabled through the debugfs load_tracker file.
> > +                */
> > +               vc4->load_tracker_enabled = 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 = 2048;
> > -       dev->mode_config.max_height = 2048;
> > +       if (of_device_is_compatible(dev->dev->of_node, "brcm,bcm2711-vc5")) {
> 
> 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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)