On Wed, Jul 11, 2018 at 05:57:08PM +0000, C, Ramalingam wrote: > Thanks seanpaul for the reviews. > > > -----Original Message----- > > From: Sean Paul [mailto:seanpaul@xxxxxxxxxxxx] > > Sent: Tuesday, July 10, 2018 1:51 AM > > To: C, Ramalingam <ramalingam.c@xxxxxxxxx> > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > > daniel@xxxxxxxx; Winkler, Tomas <tomas.winkler@xxxxxxxxx>; Usyskin, > > Alexander <alexander.usyskin@xxxxxxxxx>; Shankar, Uma > > <uma.shankar@xxxxxxxxx> > > Subject: Re: [PATCH v5 01/40] drm: hdcp2.2 authentication msg > > definitions > > > > On Wed, Jun 27, 2018 at 02:09:50PM +0530, Ramalingam C wrote: > > > This patch defines the hdcp2.2 protocol messages for authentication. > > > > > > v2: > > > bit_fields are removed. Instead bitmasking used. [Tomas and Jani] > > > prefix HDCP_2_2_ is added to the macros. [Tomas] > > > v3: > > > No Changes. > > > v4: > > > Style and spellings are fixed [Uma] > > > v5: > > > Fix for macros. > > > > > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx> > > > --- > > > include/drm/drm_hdcp.h | 179 > > > +++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 179 insertions(+) > > > > > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index > > > 98e63d870139..3e963c5d04b2 100644 > > > --- a/include/drm/drm_hdcp.h > > > +++ b/include/drm/drm_hdcp.h > > > @@ -38,4 +38,183 @@ > > > #define DRM_HDCP_DDC_BSTATUS 0x41 > > > #define DRM_HDCP_DDC_KSV_FIFO 0x43 > > > > > > +#define DRM_HDCP_1_4_SRM_ID 0x8 > > > +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE 3 > > > +#define DRM_HDCP_1_4_DCP_SIG_SIZE 40 > > > > These don't seem to be related to the patch? > > > > > + > > > +/* Protocol message definition for HDCP2.2 specification */ > > > +#define HDCP_STREAM_TYPE0 0x00 > > > +#define HDCP_STREAM_TYPE1 0x01 > > > > Why not HDCP_2_2 prefix? > Though they are introduced at HDCP2.2, this is classification of the streams. > And Type 0 can be transmitted on HDCP1.4. > So keeping it as generic name with no version mentioned. Ok, I guess it's the comment that was throwing me off. Perhaps you could improve it to: /* * Protected video streams are classified into 2 types: * - Type0: Can be transmitted with HDCP 1.4+ * - Type1: Can be transmitted with HDCP 2.2+ */ /snip > > > +} __packed; > > > > Perhaps this has already been asked and answered, but do all of these need to > > be __packed? This is kind of the problem with adding a bunch of unused > > structures to a patch, it's hard to see what their usage is. In future, these should > > probably be introduced when they're being used. > > These are the HDCP2.2 message defined at HDCP2.2 spec. And they needs to be > __packed just to have exact size mentioned by spec. > > Like how we have HDCP1.4 and 2.2 macros defined as per the HDCP spec definitions, > defined the HDCP2.2 messages together here. Thanks for the explanation. Sean > > Thanks, > Ram. > > > > Sean > > > > > + > > > #endif > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Sean Paul, Software Engineer, Google / Chromium OS -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
