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

Manasi Navare manasi.d.navare at intel.com
Fri Aug 3 19:18:42 UTC 2018


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?
I am adding newline here after the operator if the next operand exceeds the 80 column limit.

> 
> > +               (u8)dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
> > +               (u8)dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
> > +               (u8)dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
> > +               (u8)dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;
> 
> 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.
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?

Manasi

> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list