[PATCH v2 10/23] drm/dsc: Add helpers for DSC picture parameter set infoframes

Chris Wilson chris at chris-wilson.co.uk
Fri Aug 3 19:43:51 UTC 2018


Quoting Manasi Navare (2018-08-03 20:18:42)
> On Tue, Jul 31, 2018 at 10:16:45PM +0100, Chris Wilson wrote:
> > Quoting Manasi Navare (2018-07-31 22:07:06)
> > > +       /* PPS 4 */
> > > +       pps_sdp->pps_payload.pps_4 = (u8)((dsc_cfg->bits_per_pixel &
> > > +                                          DSC_PPS_BPP_HIGH_MASK) >>
> > > +                                         DSC_PPS_MSB_SHIFT) |
> > 
> > To avoid overhanging cliffs, insert the newline after the sequence
> > point. Quite a few examples throughout the series that would benefit
> > from more judicial placement of line breaks.
> 
> What exactly are you refering to by sequence point here?

Which ever makes sense visually, here '='.

pps_sdp->pps_payload.pps_4 =
	dsc_cfg->bits_per_pixel >> X << DSC_PPS_MSB_SHIFT |
	dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
	dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
	dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
	dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;

If you want to mask with 0xff do it without having to refer to C type promotion
rules.

> > Furthermore, you only need the SPDX shorthand rather than full licence
> > text.
> 
> Here the header is what all the other .c files in drm have. They all tend to use
> complete text.

And they all need updating, eventually. There's no reason for new file
to use the old style.

> Are you suggesting just adding SPDX-License-Identifier: GPL-2.0+ at the begining of the file?
> Should I remove the entire paragraph about the Copyright?

No, the code is MIT. You replace the licence with just a SPDX link.
-Chris


More information about the dri-devel mailing list