Re: [PATCH v3 4/4] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[Nicolas Frattaroli]

On Tuesday, 26 May 2026 12:52:31 Central European Summer Time Jani Nikula wrote:
> On Tue, 26 May 2026, Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx> wrote:
> > HDMI 2.1 redefines previously reserved fields in SCDC for various new
> > uses. No version check needs to be performed, as an HDMI 2.0 sink's
> > reserved SCDC fields are well-defined to be 0, and any zero-ness of
> > these fields for an HDMI 2.0 sink is not a surprise for SCDC parsers for
> > HDMI 2.1.
> >
> > Implement reading and outputting these fields over debugfs.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/display/drm_scdc_helper.c | 154 ++++++++++++++++++++++++++----
> >  include/drm/display/drm_scdc.h            |   9 ++
> >  include/drm/display/drm_scdc_helper.h     |  85 ++++++++++++++---
> >  3 files changed, 218 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> > index 75a59c6fc7a5..b5a866372b0f 100644
> > --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> > +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> > @@ -21,6 +21,7 @@
> >   * DEALINGS IN THE SOFTWARE.
> >   */
> >  
> > +#include <linux/bitfield.h>
> >  #include <linux/export.h>
> >  #include <linux/i2c.h>
> >  #include <linux/slab.h>
> > @@ -63,6 +64,38 @@ struct scdc_debugfs_priv {
> >  	struct drm_scdc_state state;
> >  };
> >  
> > +static __pure const char *drm_scdc_frl_rate_str(enum drm_scdc_frl_rate rate)
> 
> There are a total of only 71 __pure usages in the kernel. Is this
> helpful for a static function where the compiler can figure it out by
> itself?

For the compiler, no. For the programmer, maybe. It's basically
documentation that foo(x) remains the same for a specific x, so
that having 3 calls with the same parameter in some scope is not
actually 3 separate function calls, but just retrieving a value
without manually saving it into a stack variable. A compiler can
infer that in this case, but a programmer that has their LSP show
them the function signature on hover will immediately know what's
going on without having to look at the implementation.

I have a habit of slapping it onto any function that is pure just
because, but if you'd rather have me drop it then I can do that.
I'm aware that slapping attributes onto function signatures can
make them a bit noisy to read.

> Here too, bitfield bit order is implementation defined. See my other
> reply in the thread.

Ugh, I was hoping that with just 1-byte bitfields, this would not be
a problem. If that's not the case, then I guess Bug Endian ruins
another handy shortcut. :^( Oh well.

Thanks for the reviews, will send a new revision with the comments
addressed within the week probably.

Kind regards,
Nicolas Frattaroli