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