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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx