>-----Original Message----- >From: Navare, Manasi D >Sent: Thursday, December 21, 2017 12:36 PM >To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Ville Syrjala ><ville.syrjala@xxxxxxxxxxxxxxx>; Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> >Subject: Re: [PATCH] drm: Add DPCD definitions for DP 1.4 FEC feature > >On Mon, Nov 27, 2017 at 04:55:44PM -0800, Anusha Srivatsa wrote: >> Forward Error Correction is supported on DP 1.4. >> This patch adds corresponding DPCD register definitions. >> >> v2: Add dri-devel to the CC list >> >> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx >> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> >> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> >> --- >> include/drm/drm_dp_helper.h | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index da58a42..bc816ea 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -284,6 +284,35 @@ >> # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 >> # define DP_DSC_BITS_PER_PIXEL_1 0x4 >> >> +/* DP Forward error Correction Registers */ >> +#define DP_FEC_CAPABILITY 0x090 >> +# define DP_FEC_CAPABLE (1 << 0) >> +# define DP_FEC_UNCORR_BLK_ERROR_COUNT_CAP (1 << 1) >> +# define DP_FEC_CORR_BLK_ERROR_COUNT_CAP (1 << 2) >> +# define DP_FEC_BIT_ERROR_COUNT_CAP (1 << 3) >> + >> +#define DP_FEC_CONFIGURATION 0x120 >> +# define DP_FEC_READY (1 << 0) >> +# define DP_FEC_ERR_COUNT_DIS (0 << 1) >> +# define DP_FEC_UNCORR_BLK_ERROR_COUNT (1 << 1) >> +# define DP_FEC_CORR_BLK_ERROR_COUNT (2 << 1) >> +# define DP_FEC_BIT_ERROR_COUNT (3 << 1) > >These above values indicate the value of FEC_ERROR_COUNT_SEL. >I think we would need a mask for FEC_ERROR_COUNT_SEL field so that we can >read this field as drm_dpcd_read(DP_FEC_CONFIGURATION) & >FEC_ERROR_COUNT_SEL_MASK and then compare this to each of the values for >that field. >So we would need an extra #define for the MASK Sounds good.... >> +# define DP_FEC_LANE_0_SELECT (0 << 4) >> +# define DP_FEC_LANE_1_SELECT (1 << 4) >> +# define DP_FEC_LANE_2_SELECT (2 << 4) >> +# define DP_FEC_LANE_3_SELECT (3 << 4) >> + >> +#define DP_FEC_STATUS 0x280 >> +# define DP_FEC_EN_DETECTED (1 << 0) > >I think better name would be DP_FEC_DECODE_EN_DETECTED since this refers to >FEC_DECODE_EN link symbol sequence Now that you mentioned, I noticed that's the name used in spec too. I wonder why I went ahead with the above name. Shall change it. Thanks. >> +# define DP_FEC_DEC_DETECTED (1 << 1) > >And this should be DP_FEC_DECODE_DIS_DETECTED since this refers to >FEC_DECODE_DIS link symbol sequence > >> + >> +#define DP_FEC_ERROR_COUNT_1 0x0281 >> +# define DP_FEC_ERR_COUNT_7_0(err_count) (err_count << 0) > >So this is a RO register and so we wont be writing the err_count by passing it as >an argument as above. >And this is the entire reister that indicates the LSB of ERR_COUNT you can just >rename the register as DP_FEC_ERR_COUNT_LSB So while reading you just pass >this register address and get LSB into a variable. Yes, good point. > >> + >> +#define DP_FEC_ERROR_COUNT_2 0x0282 >> +# define DP_FEC_ERR_COUNT_14_8(err_count) (err_count << 0) > >This could be DP_FEC_ERR_COUNT_MSB_MASK and SHIFT should be 8 since you >want to put this value at the 8th bit of a 16 bit value. This helps.... thanks! >> +# define DP_FEC_ERR_COUNT_VALID (1 << 7) > >Everything else looks good. Thanks Manasi. I will incorporate these changes in the next revision. Anusha >Manasi > >> + >> #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ >> # define DP_PSR_IS_SUPPORTED 1 >> # define DP_PSR2_IS_SUPPORTED 2 /* eDP 1.4 */ >> -- >> 2.7.4 >> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
