diff mbox series

drm/dsc: Add kernel documentation for DRM DP DSC helpers

Message ID 20190204234022.4170-1-manasi.d.navare@intel.com
State New, archived
Headers show
Series drm/dsc: Add kernel documentation for DRM DP DSC helpers | expand

Commit Message

Navare, Manasi Feb. 4, 2019, 11:40 p.m. UTC
This patch adds appropiate kernel documentation for DRM DP helpers
used for enabling Display Stream compression functionality in
drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
related structure definitions and helpers in drm_dsc.c and drm_dsc.h
Also add links between the functions and structures in the documentation.

Suggested-by: Daniel Vetter <daniel.vetter@intel.com>
Suggested-by: Sean Paul <sean@poorly.run>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c |  42 +++++++++-
 drivers/gpu/drm/drm_dsc.c       |  13 ++-
 include/drm/drm_dp_helper.h     |  15 +++-
 include/drm/drm_dsc.h           | 138 ++++++++++++++++++--------------
 4 files changed, 142 insertions(+), 66 deletions(-)

Comments

Daniel Vetter Feb. 5, 2019, 9:55 a.m. UTC | #1
On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> This patch adds appropiate kernel documentation for DRM DP helpers
> used for enabling Display Stream compression functionality in
> drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> Also add links between the functions and structures in the documentation.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@intel.com>
> Suggested-by: Sean Paul <sean@poorly.run>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Sean Paul <sean@poorly.run>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>

Awesome, more docs!

> ---
>  drivers/gpu/drm/drm_dp_helper.c |  42 +++++++++-
>  drivers/gpu/drm/drm_dsc.c       |  13 ++-
>  include/drm/drm_dp_helper.h     |  15 +++-
>  include/drm/drm_dsc.h           | 138 ++++++++++++++++++--------------
>  4 files changed, 142 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 54120b6319e7..e9e0233f5b2f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
>  EXPORT_SYMBOL(drm_dp_read_desc);
>  
>  /**
> - * DRM DP Helpers for DSC
> + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> + * supported by the DSC sink.
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @is_edp: true if its eDP, false for DP
> + *
> + * Read the slice capabilities DPCD register from DSC sink to get
> + * the maximum slice count supported. This is used to populate
> + * the DSC parameters in the &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + *
> + * Returns:
> + * Maximum slice count supported by DSC sink or 0 its invalid
>   */
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  				   bool is_edp)
> @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
>  
> +/**
> + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits which is
> + * number of bits of precision within the decoder line buffer supported by
> + * the DSC sink. This is used to populate the DSC parameters in the
> + * &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()

The entire above block ends up being used as the "one-line" summary, and
no description. I guess you wanted to split it up like the one above?

The description starts after the first empty newline, everything before
that is the summary.

> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns:
> + * Line buffer depth supported by DSC panel or 0 its invalid
> + */
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  {
>  	u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - DP_DSC_SUPPORT];
> @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> +/**
> + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per component
> + * values supported by the DSC sink. This is used to populate the DSC parameters
> + * in the &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @dsc_bpc: An array to be filled by this helper with supported
> + *           input bpcs.
> + *
> + * Returns:
> + * Number of input BPC values parsed from the DPCD
> + */
>  int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  					 u8 dsc_bpc[3])
>  {
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index bc2b23adb072..0fd56fbdf9b4 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -17,6 +17,12 @@
>  /**
>   * DOC: dsc helpers
>   *
> + * VESA specification for DP 1.4 adds a new feature called Display Stream
> + * Compression (DSC) used to compress the pixel bits before sending it on
> + * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the existing
> + * display interfaces can support high resolutions at higher frames rates uisng
> + * the maximum available link capacity of these interfaces.
> + *
>   * These functions contain some common logic and helpers to deal with VESA
>   * Display Stream Compression standard required for DSC on Display Port/eDP or
>   * MIPI display interfaces.
> @@ -26,6 +32,7 @@
>   * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
>   * for DisplayPort as per the DP 1.4 spec.
>   * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
> + *           as defined in &struct drm_dsc_pps_infoframe
>   */
>  void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
>  {
> @@ -44,9 +51,11 @@ EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
>   * that span more than 1 byte.
>   *
>   * @pps_sdp:
> - * Secondary data packet for DSC Picture Parameter Set
> + * Secondary data packet for DSC Picture Parameter Set. This is defined
> + * by &struct drm_dsc_pps_infoframe
>   * @dsc_cfg:
> - * DSC Configuration data filled by driver
> + * DSC Configuration data filled by driver as defined by
> + * &struct drm_dsc_config
>   */
>  void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  				const struct drm_dsc_config *dsc_cfg)
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5db7fb8c8b50..2711cdfa0c13 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1052,11 +1052,18 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw);
>  #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
>  /* 0x80+ CEA-861 infoframe types */
>  
> +/**
> + * struct dp_sdp_header - DP secondary data packet header
> + * @HB0: Secondary Data Packet ID
> + * @HB1: Secondary Data Packet Type
> + * @HB2: Secondary Data Packet Specific header, Byte 0
> + * @HB3: Secondary Data packet Specific header, Byte 1
> + */
>  struct dp_sdp_header {
> -	u8 HB0; /* Secondary Data Packet ID */
> -	u8 HB1; /* Secondary Data Packet Type */
> -	u8 HB2; /* Secondary Data Packet Specific header, Byte 0 */
> -	u8 HB3; /* Secondary Data packet Specific header, Byte 1 */
> +	u8 HB0;
> +	u8 HB1;
> +	u8 HB2;
> +	u8 HB3;
>  } __packed;
>  
>  #define EDP_SDP_HEADER_REVISION_MASK		0x1F
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index d03f1b83421a..f50d265a97e2 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -44,111 +44,128 @@
>  #define DSC_1_2_MAX_LINEBUF_DEPTH_VAL		0
>  #define DSC_1_1_MAX_LINEBUF_DEPTH_BITS		13
>  
> -/* Configuration for a single Rate Control model range */
> +/**
> + * struct drm_dsc_rc_range_parameters - DSC Rate Control range parameters
> + *
> + * @range_min_qp: Min Quantization Parameters allowed for this range
> + * @range_max_qp: Max Quantization Parameters allowed for this range
> + * @range_bpg_offset: Bits/group offset to apply to target for this group
> + */
>  struct drm_dsc_rc_range_parameters {
> -	/* Min Quantization Parameters allowed for this range */
>  	u8 range_min_qp;
> -	/* Max Quantization Parameters allowed for this range */
>  	u8 range_max_qp;
> -	/* Bits/group offset to apply to target for this group */
>  	u8 range_bpg_offset;

Since you already have inline comments I'd stick to inline kerneldoc.
Those can also be just one line, e.g.

	/** foo: one-line description of what foo is */
	u8 foo;

>  };
>  
> +/**
> + * struct drm_dsc_config - Parameters required to configure DSC
> + *
> + * @line_buf_depth: Bits / component for previous reconstructed line buffer
> + * @bits_per_component: Bits per component to code (8/10/12)
> + * @convert_rgb: Flag to indicate if RGB - YCoCg conversion is needed
> + *               True if RGB input, False if YCoCg input
> + * @slice_count: Number fo slices per line used by the DSC encoder
> + * @slice_width: Width of each slice in pixels
> + * @slice_height: Slice height in pixels
> + * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
> + * @pic_width: Width of the input display frame in pixels
> + * @pic_height: Vertical height of the input display frame
> + * @rc_tgt_offset_high: Offset to bits/group used by RC to determine QP
> + *                      adjustment
> + * @rc_tgt_offset_low: Offset to bits/group used by RC to determine QP
> + *                     adjustment
> + * @bits_per_pixel: Target bits per pixel with 4 fractional bits.
> + *                  bits_per_pixel << 4
> + * @rc_edge_factor: Factor to determine if an edge is present based on the
> + *                  bits produced
> + * @rc_quant_incr_limit1: Slow down incrementing once the range reaches this
> + *                        value
> + * @rc_quant_incr_limit0: Slow down incrementing once the range reaches this
> + *                        value
> + * @initial_xmit_delay: Number of pixels to delay the initial transmission
> + * @initial_dec_delay: Initial decoder delay, number of pixel times that the
> + *                     decoer accumulates data in its rate buffer before
> + *                     starting to decode and output pixels.
> + * @block_pred_enable: True if block prediction is used to code any groups
> + *                     within the picture.
> + *                     False if BP not used
> + * @first_line_bpg_offset: Number of additional bits allocated for each group on
> + *                         the first line of slice.
> + * @initial_offset: Value to use for RC model offset at slice start
> + * @rc_buf_thresh: Thresholds defining each of the buffer ranges
> + * @rc_range_params: Parameters for each of the RC ranges defined in
> + *                   &struct drm_dsc_rc_range_parameters
> + * @rc_model_size: Total size of RC model
> + * @flatness_min_qp: Minimum QP where flatness information is sent
> + * @flatness_max_qp: Maximum QP where flatness information is sent
> + * @initial_scale_value: Initial value for the scale factor
> + * @scale_decrement_interval: Specifies number of group times between
> + *                            decrementing the scale factor at beginning
> + *                            of a slice.
> + * @scale_increment_interval: Number of group times between incrementing
> + *                            the scale factor value used at the beginning
> + *                            of a slice.
> + * @nfl_bpg_offset: Non first line BPG offset to be used
> + * @slice_bpg_offset: BPG offset used to enforce slice bit
> + * @final_offset: Final RC linear transformation offset value
> + * @vbr_enable: True if VBR mode is enabled, false if disabled
> + * @mux_word_size: Mux word size (in bits) for SSM mode
> + * @slice_chunk_size: The (max) size in bytes of the "chunks" that are
> + *                    used in slice multiplexing.
> + * @rc_bits: Rate control buffer size in bits
> + * @dsc_version_minor: DSC minor version
> + * @dsc_version_major: DSC major version
> + * @native_422: True if Native 4:2:2 supported, else false
> + * @native_420: True if Native 4:2:0 supported else false.
> + * @second_line_bpg_offset: Additional bits/grp for seconnd line of slice for
> + *                          native 4:2:0
> + * @nsl_bpg_offset: Num of bits deallocated for each grp that is not in second
> + *                  line of slice
> + * @second_line_offset_adj: Offset adjustment for second line in Native 4:2:0
> + *                          mode

Yeah I think this is a clear example of where struct comments all in the
top is much worse than inline comments. Please go with the inline style
here.

Cheers, Daniel

> + */
>  struct drm_dsc_config {
> -	/* Bits / component for previous reconstructed line buffer */
>  	u8 line_buf_depth;
> -	/* Bits per component to code (must be 8, 10, or 12) */
>  	u8 bits_per_component;
> -	/*
> -	 * Flag indicating to do RGB - YCoCg conversion
> -	 * and back (should be 1 for RGB input)
> -	 */
>  	bool convert_rgb;
>  	u8 slice_count;
> -	/* Slice Width */
>  	u16 slice_width;
> -	/* Slice Height */
>  	u16 slice_height;
> -	/*
> -	 * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
> -	 * outside of DSC encode/decode algorithm)
> -	 */
>  	bool enable422;
> -	/* Picture Width */
>  	u16 pic_width;
> -	/* Picture Height */
>  	u16 pic_height;
> -	/* Offset to bits/group used by RC to determine QP adjustment */
>  	u8 rc_tgt_offset_high;
> -	/* Offset to bits/group used by RC to determine QP adjustment */
>  	u8 rc_tgt_offset_low;
> -	/* Bits/pixel target << 4 (ie., 4 fractional bits) */
>  	u16 bits_per_pixel;
> -	/*
> -	 * Factor to determine if an edge is present based
> -	 * on the bits produced
> -	 */
>  	u8 rc_edge_factor;
> -	/* Slow down incrementing once the range reaches this value */
>  	u8 rc_quant_incr_limit1;
> -	/* Slow down incrementing once the range reaches this value */
>  	u8 rc_quant_incr_limit0;
> -	/* Number of pixels to delay the initial transmission */
>  	u16 initial_xmit_delay;
> -	/* Number of pixels to delay the VLD on the decoder,not including SSM */
>  	u16  initial_dec_delay;
> -	/* Block prediction enable */
>  	bool block_pred_enable;
> -	/* Bits/group offset to use for first line of the slice */
>  	u8 first_line_bpg_offset;
> -	/* Value to use for RC model offset at slice start */
>  	u16 initial_offset;
> -	/* Thresholds defining each of the buffer ranges */
>  	u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1];
> -	/* Parameters for each of the RC ranges */
>  	struct drm_dsc_rc_range_parameters rc_range_params[DSC_NUM_BUF_RANGES];
> -	/* Total size of RC model */
>  	u16 rc_model_size;
> -	/* Minimum QP where flatness information is sent */
>  	u8 flatness_min_qp;
> -	/* Maximum QP where flatness information is sent */
>  	u8 flatness_max_qp;
> -	/* Initial value for scale factor */
>  	u8 initial_scale_value;
> -	/* Decrement scale factor every scale_decrement_interval groups */
>  	u16 scale_decrement_interval;
> -	/* Increment scale factor every scale_increment_interval groups */
>  	u16 scale_increment_interval;
> -	/* Non-first line BPG offset to use */
>  	u16 nfl_bpg_offset;
> -	/* BPG offset used to enforce slice bit */
>  	u16 slice_bpg_offset;
> -	/* Final RC linear transformation offset value */
>  	u16 final_offset;
> -	/* Enable on-off VBR (ie., disable stuffing bits) */
>  	bool vbr_enable;
> -	/* Mux word size (in bits) for SSM mode */
>  	u8 mux_word_size;
> -	/*
> -	 * The (max) size in bytes of the "chunks" that are
> -	 * used in slice multiplexing
> -	 */
>  	u16 slice_chunk_size;
> -	/* Rate Control buffer siz in bits */
>  	u16 rc_bits;
> -	/* DSC Minor Version */
>  	u8 dsc_version_minor;
> -	/* DSC Major version */
>  	u8 dsc_version_major;
> -	/* Native 4:2:2 support */
>  	bool native_422;
> -	/* Native 4:2:0 support */
>  	bool native_420;
> -	/* Additional bits/grp for seconnd line of slice for native 4:2:0 */
>  	u8 second_line_bpg_offset;
> -	/* Num of bits deallocated for each grp that is not in second line of slice */
>  	u16 nsl_bpg_offset;
> -	/* Offset adj fr second line in Native 4:2:0 mode */
>  	u16 second_line_offset_adj;
>  };
>  
> @@ -468,10 +485,13 @@ struct drm_dsc_picture_parameter_set {
>   * This structure represents the DSC PPS infoframe required to send the Picture
>   * Parameter Set metadata required before enabling VESA Display Stream
>   * Compression. This is based on the DP Secondary Data Packet structure and
> - * comprises of SDP Header as defined in drm_dp_helper.h and PPS payload.
> + * comprises of SDP Header as defined &struct struct dp_sdp_header in drm_dp_helper.h
> + * and PPS payload defined in &struct drm_dsc_picture_parameter_set.
>   *
> - * @pps_header: Header for PPS as per DP SDP header format
> + * @pps_header: Header for PPS as per DP SDP header format of type
> + *              &struct dp_sdp_header
>   * @pps_payload: PPS payload fields as per DSC specification Table 4-1
> + *               as represented in &struct drm_dsc_picture_parameter_set
>   */
>  struct drm_dsc_pps_infoframe {
>  	struct dp_sdp_header pps_header;
> -- 
> 2.19.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Navare, Manasi Feb. 5, 2019, 5:44 p.m. UTC | #2
On Tue, Feb 05, 2019 at 10:55:12AM +0100, Daniel Vetter wrote:
> On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> > This patch adds appropiate kernel documentation for DRM DP helpers
> > used for enabling Display Stream compression functionality in
> > drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> > related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> > Also add links between the functions and structures in the documentation.
> > 
> > Suggested-by: Daniel Vetter <daniel.vetter@intel.com>
> > Suggested-by: Sean Paul <sean@poorly.run>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> Awesome, more docs!
> 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c |  42 +++++++++-
> >  drivers/gpu/drm/drm_dsc.c       |  13 ++-
> >  include/drm/drm_dp_helper.h     |  15 +++-
> >  include/drm/drm_dsc.h           | 138 ++++++++++++++++++--------------
> >  4 files changed, 142 insertions(+), 66 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 54120b6319e7..e9e0233f5b2f 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
> >  EXPORT_SYMBOL(drm_dp_read_desc);
> >  
> >  /**
> > - * DRM DP Helpers for DSC
> > + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> > + * supported by the DSC sink.
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + * @is_edp: true if its eDP, false for DP
> > + *
> > + * Read the slice capabilities DPCD register from DSC sink to get
> > + * the maximum slice count supported. This is used to populate
> > + * the DSC parameters in the &struct drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> > + *
> > + * Returns:
> > + * Maximum slice count supported by DSC sink or 0 its invalid
> >   */
> >  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  				   bool is_edp)
> > @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
> >  
> > +/**
> > + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits which is
> > + * number of bits of precision within the decoder line buffer supported by
> > + * the DSC sink. This is used to populate the DSC parameters in the
> > + * &struct drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> 
> The entire above block ends up being used as the "one-line" summary, and
> no description. I guess you wanted to split it up like the one above?
> 
> The description starts after the first empty newline, everything before
> that is the summary.

Yes I will split this as summary and description. I tested this in html docs
but my eyes failed to catch this. Thanks for pointing out.

> 
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + *
> > + * Returns:
> > + * Line buffer depth supported by DSC panel or 0 its invalid
> > + */
> >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  {
> >  	u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - DP_DSC_SUPPORT];
> > @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> >  
> > +/**
> > + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per component
> > + * values supported by the DSC sink. This is used to populate the DSC parameters
> > + * in the &struct drm_dsc_config by the driver.
> > + * Driver creates an infoframe using these parameters to populate
> > + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> > + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> > + * @dsc_dpcd: DSC capabilities from DPCD
> > + * @dsc_bpc: An array to be filled by this helper with supported
> > + *           input bpcs.
> > + *
> > + * Returns:
> > + * Number of input BPC values parsed from the DPCD
> > + */
> >  int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  					 u8 dsc_bpc[3])
> >  {
> > diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> > index bc2b23adb072..0fd56fbdf9b4 100644
> > --- a/drivers/gpu/drm/drm_dsc.c
> > +++ b/drivers/gpu/drm/drm_dsc.c
> > @@ -17,6 +17,12 @@
> >  /**
> >   * DOC: dsc helpers
> >   *
> > + * VESA specification for DP 1.4 adds a new feature called Display Stream
> > + * Compression (DSC) used to compress the pixel bits before sending it on
> > + * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the existing
> > + * display interfaces can support high resolutions at higher frames rates uisng
> > + * the maximum available link capacity of these interfaces.
> > + *
> >   * These functions contain some common logic and helpers to deal with VESA
> >   * Display Stream Compression standard required for DSC on Display Port/eDP or
> >   * MIPI display interfaces.
> > @@ -26,6 +32,7 @@
> >   * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
> >   * for DisplayPort as per the DP 1.4 spec.
> >   * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
> > + *           as defined in &struct drm_dsc_pps_infoframe
> >   */
> >  void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
> >  {
> > @@ -44,9 +51,11 @@ EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
> >   * that span more than 1 byte.
> >   *
> >   * @pps_sdp:
> > - * Secondary data packet for DSC Picture Parameter Set
> > + * Secondary data packet for DSC Picture Parameter Set. This is defined
> > + * by &struct drm_dsc_pps_infoframe
> >   * @dsc_cfg:
> > - * DSC Configuration data filled by driver
> > + * DSC Configuration data filled by driver as defined by
> > + * &struct drm_dsc_config
> >   */
> >  void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> >  				const struct drm_dsc_config *dsc_cfg)
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 5db7fb8c8b50..2711cdfa0c13 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1052,11 +1052,18 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw);
> >  #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
> >  /* 0x80+ CEA-861 infoframe types */
> >  
> > +/**
> > + * struct dp_sdp_header - DP secondary data packet header
> > + * @HB0: Secondary Data Packet ID
> > + * @HB1: Secondary Data Packet Type
> > + * @HB2: Secondary Data Packet Specific header, Byte 0
> > + * @HB3: Secondary Data packet Specific header, Byte 1
> > + */
> >  struct dp_sdp_header {
> > -	u8 HB0; /* Secondary Data Packet ID */
> > -	u8 HB1; /* Secondary Data Packet Type */
> > -	u8 HB2; /* Secondary Data Packet Specific header, Byte 0 */
> > -	u8 HB3; /* Secondary Data packet Specific header, Byte 1 */
> > +	u8 HB0;
> > +	u8 HB1;
> > +	u8 HB2;
> > +	u8 HB3;
> >  } __packed;

Making this a packed struct should be in a separate patch right?
Functional change.

> >  
> >  #define EDP_SDP_HEADER_REVISION_MASK		0x1F
> > diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> > index d03f1b83421a..f50d265a97e2 100644
> > --- a/include/drm/drm_dsc.h
> > +++ b/include/drm/drm_dsc.h
> > @@ -44,111 +44,128 @@
> >  #define DSC_1_2_MAX_LINEBUF_DEPTH_VAL		0
> >  #define DSC_1_1_MAX_LINEBUF_DEPTH_BITS		13
> >  
> > -/* Configuration for a single Rate Control model range */
> > +/**
> > + * struct drm_dsc_rc_range_parameters - DSC Rate Control range parameters
> > + *
> > + * @range_min_qp: Min Quantization Parameters allowed for this range
> > + * @range_max_qp: Max Quantization Parameters allowed for this range
> > + * @range_bpg_offset: Bits/group offset to apply to target for this group
> > + */
> >  struct drm_dsc_rc_range_parameters {
> > -	/* Min Quantization Parameters allowed for this range */
> >  	u8 range_min_qp;
> > -	/* Max Quantization Parameters allowed for this range */
> >  	u8 range_max_qp;
> > -	/* Bits/group offset to apply to target for this group */
> >  	u8 range_bpg_offset;
> 
> Since you already have inline comments I'd stick to inline kerneldoc.
> Those can also be just one line, e.g.
> 
> 	/** foo: one-line description of what foo is */
> 	u8 foo;
>

Ok will change it back
 
> >  };
> >  
> > +/**
> > + * struct drm_dsc_config - Parameters required to configure DSC
> > + *
> > + * @line_buf_depth: Bits / component for previous reconstructed line buffer
> > + * @bits_per_component: Bits per component to code (8/10/12)
> > + * @convert_rgb: Flag to indicate if RGB - YCoCg conversion is needed
> > + *               True if RGB input, False if YCoCg input
> > + * @slice_count: Number fo slices per line used by the DSC encoder
> > + * @slice_width: Width of each slice in pixels
> > + * @slice_height: Slice height in pixels
> > + * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
> > + * @pic_width: Width of the input display frame in pixels
> > + * @pic_height: Vertical height of the input display frame
> > + * @rc_tgt_offset_high: Offset to bits/group used by RC to determine QP
> > + *                      adjustment
> > + * @rc_tgt_offset_low: Offset to bits/group used by RC to determine QP
> > + *                     adjustment
> > + * @bits_per_pixel: Target bits per pixel with 4 fractional bits.
> > + *                  bits_per_pixel << 4
> > + * @rc_edge_factor: Factor to determine if an edge is present based on the
> > + *                  bits produced
> > + * @rc_quant_incr_limit1: Slow down incrementing once the range reaches this
> > + *                        value
> > + * @rc_quant_incr_limit0: Slow down incrementing once the range reaches this
> > + *                        value
> > + * @initial_xmit_delay: Number of pixels to delay the initial transmission
> > + * @initial_dec_delay: Initial decoder delay, number of pixel times that the
> > + *                     decoer accumulates data in its rate buffer before
> > + *                     starting to decode and output pixels.
> > + * @block_pred_enable: True if block prediction is used to code any groups
> > + *                     within the picture.
> > + *                     False if BP not used
> > + * @first_line_bpg_offset: Number of additional bits allocated for each group on
> > + *                         the first line of slice.
> > + * @initial_offset: Value to use for RC model offset at slice start
> > + * @rc_buf_thresh: Thresholds defining each of the buffer ranges
> > + * @rc_range_params: Parameters for each of the RC ranges defined in
> > + *                   &struct drm_dsc_rc_range_parameters
> > + * @rc_model_size: Total size of RC model
> > + * @flatness_min_qp: Minimum QP where flatness information is sent
> > + * @flatness_max_qp: Maximum QP where flatness information is sent
> > + * @initial_scale_value: Initial value for the scale factor
> > + * @scale_decrement_interval: Specifies number of group times between
> > + *                            decrementing the scale factor at beginning
> > + *                            of a slice.
> > + * @scale_increment_interval: Number of group times between incrementing
> > + *                            the scale factor value used at the beginning
> > + *                            of a slice.
> > + * @nfl_bpg_offset: Non first line BPG offset to be used
> > + * @slice_bpg_offset: BPG offset used to enforce slice bit
> > + * @final_offset: Final RC linear transformation offset value
> > + * @vbr_enable: True if VBR mode is enabled, false if disabled
> > + * @mux_word_size: Mux word size (in bits) for SSM mode
> > + * @slice_chunk_size: The (max) size in bytes of the "chunks" that are
> > + *                    used in slice multiplexing.
> > + * @rc_bits: Rate control buffer size in bits
> > + * @dsc_version_minor: DSC minor version
> > + * @dsc_version_major: DSC major version
> > + * @native_422: True if Native 4:2:2 supported, else false
> > + * @native_420: True if Native 4:2:0 supported else false.
> > + * @second_line_bpg_offset: Additional bits/grp for seconnd line of slice for
> > + *                          native 4:2:0
> > + * @nsl_bpg_offset: Num of bits deallocated for each grp that is not in second
> > + *                  line of slice
> > + * @second_line_offset_adj: Offset adjustment for second line in Native 4:2:0
> > + *                          mode
> 
> Yeah I think this is a clear example of where struct comments all in the
> top is much worse than inline comments. Please go with the inline style
> here.
> 
> Cheers, Daniel
>

Okay will change all these back in the inline commenting format.

Manasi
 
> > + */
> >  struct drm_dsc_config {
> > -	/* Bits / component for previous reconstructed line buffer */
> >  	u8 line_buf_depth;
> > -	/* Bits per component to code (must be 8, 10, or 12) */
> >  	u8 bits_per_component;
> > -	/*
> > -	 * Flag indicating to do RGB - YCoCg conversion
> > -	 * and back (should be 1 for RGB input)
> > -	 */
> >  	bool convert_rgb;
> >  	u8 slice_count;
> > -	/* Slice Width */
> >  	u16 slice_width;
> > -	/* Slice Height */
> >  	u16 slice_height;
> > -	/*
> > -	 * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
> > -	 * outside of DSC encode/decode algorithm)
> > -	 */
> >  	bool enable422;
> > -	/* Picture Width */
> >  	u16 pic_width;
> > -	/* Picture Height */
> >  	u16 pic_height;
> > -	/* Offset to bits/group used by RC to determine QP adjustment */
> >  	u8 rc_tgt_offset_high;
> > -	/* Offset to bits/group used by RC to determine QP adjustment */
> >  	u8 rc_tgt_offset_low;
> > -	/* Bits/pixel target << 4 (ie., 4 fractional bits) */
> >  	u16 bits_per_pixel;
> > -	/*
> > -	 * Factor to determine if an edge is present based
> > -	 * on the bits produced
> > -	 */
> >  	u8 rc_edge_factor;
> > -	/* Slow down incrementing once the range reaches this value */
> >  	u8 rc_quant_incr_limit1;
> > -	/* Slow down incrementing once the range reaches this value */
> >  	u8 rc_quant_incr_limit0;
> > -	/* Number of pixels to delay the initial transmission */
> >  	u16 initial_xmit_delay;
> > -	/* Number of pixels to delay the VLD on the decoder,not including SSM */
> >  	u16  initial_dec_delay;
> > -	/* Block prediction enable */
> >  	bool block_pred_enable;
> > -	/* Bits/group offset to use for first line of the slice */
> >  	u8 first_line_bpg_offset;
> > -	/* Value to use for RC model offset at slice start */
> >  	u16 initial_offset;
> > -	/* Thresholds defining each of the buffer ranges */
> >  	u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1];
> > -	/* Parameters for each of the RC ranges */
> >  	struct drm_dsc_rc_range_parameters rc_range_params[DSC_NUM_BUF_RANGES];
> > -	/* Total size of RC model */
> >  	u16 rc_model_size;
> > -	/* Minimum QP where flatness information is sent */
> >  	u8 flatness_min_qp;
> > -	/* Maximum QP where flatness information is sent */
> >  	u8 flatness_max_qp;
> > -	/* Initial value for scale factor */
> >  	u8 initial_scale_value;
> > -	/* Decrement scale factor every scale_decrement_interval groups */
> >  	u16 scale_decrement_interval;
> > -	/* Increment scale factor every scale_increment_interval groups */
> >  	u16 scale_increment_interval;
> > -	/* Non-first line BPG offset to use */
> >  	u16 nfl_bpg_offset;
> > -	/* BPG offset used to enforce slice bit */
> >  	u16 slice_bpg_offset;
> > -	/* Final RC linear transformation offset value */
> >  	u16 final_offset;
> > -	/* Enable on-off VBR (ie., disable stuffing bits) */
> >  	bool vbr_enable;
> > -	/* Mux word size (in bits) for SSM mode */
> >  	u8 mux_word_size;
> > -	/*
> > -	 * The (max) size in bytes of the "chunks" that are
> > -	 * used in slice multiplexing
> > -	 */
> >  	u16 slice_chunk_size;
> > -	/* Rate Control buffer siz in bits */
> >  	u16 rc_bits;
> > -	/* DSC Minor Version */
> >  	u8 dsc_version_minor;
> > -	/* DSC Major version */
> >  	u8 dsc_version_major;
> > -	/* Native 4:2:2 support */
> >  	bool native_422;
> > -	/* Native 4:2:0 support */
> >  	bool native_420;
> > -	/* Additional bits/grp for seconnd line of slice for native 4:2:0 */
> >  	u8 second_line_bpg_offset;
> > -	/* Num of bits deallocated for each grp that is not in second line of slice */
> >  	u16 nsl_bpg_offset;
> > -	/* Offset adj fr second line in Native 4:2:0 mode */
> >  	u16 second_line_offset_adj;
> >  };
> >  
> > @@ -468,10 +485,13 @@ struct drm_dsc_picture_parameter_set {
> >   * This structure represents the DSC PPS infoframe required to send the Picture
> >   * Parameter Set metadata required before enabling VESA Display Stream
> >   * Compression. This is based on the DP Secondary Data Packet structure and
> > - * comprises of SDP Header as defined in drm_dp_helper.h and PPS payload.
> > + * comprises of SDP Header as defined &struct struct dp_sdp_header in drm_dp_helper.h
> > + * and PPS payload defined in &struct drm_dsc_picture_parameter_set.
> >   *
> > - * @pps_header: Header for PPS as per DP SDP header format
> > + * @pps_header: Header for PPS as per DP SDP header format of type
> > + *              &struct dp_sdp_header
> >   * @pps_payload: PPS payload fields as per DSC specification Table 4-1
> > + *               as represented in &struct drm_dsc_picture_parameter_set
> >   */
> >  struct drm_dsc_pps_infoframe {
> >  	struct dp_sdp_header pps_header;
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sean Paul Feb. 5, 2019, 7:28 p.m. UTC | #3
On Mon, Feb 04, 2019 at 03:40:22PM -0800, Manasi Navare wrote:
> This patch adds appropiate kernel documentation for DRM DP helpers
> used for enabling Display Stream compression functionality in
> drm_dp_helper.h and drm_dp_helper.c as well as for the DSC spec
> related structure definitions and helpers in drm_dsc.c and drm_dsc.h
> Also add links between the functions and structures in the documentation.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@intel.com>
> Suggested-by: Sean Paul <sean@poorly.run>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Sean Paul <sean@poorly.run>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>

Build warnings are gone with this, so once Daniel's suggestions are
incorporated:

Acked-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/drm_dp_helper.c |  42 +++++++++-
>  drivers/gpu/drm/drm_dsc.c       |  13 ++-
>  include/drm/drm_dp_helper.h     |  15 +++-
>  include/drm/drm_dsc.h           | 138 ++++++++++++++++++--------------
>  4 files changed, 142 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 54120b6319e7..e9e0233f5b2f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1360,7 +1360,20 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
>  EXPORT_SYMBOL(drm_dp_read_desc);
>  
>  /**
> - * DRM DP Helpers for DSC
> + * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> + * supported by the DSC sink.
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @is_edp: true if its eDP, false for DP
> + *
> + * Read the slice capabilities DPCD register from DSC sink to get
> + * the maximum slice count supported. This is used to populate
> + * the DSC parameters in the &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + *
> + * Returns:
> + * Maximum slice count supported by DSC sink or 0 its invalid
>   */
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  				   bool is_edp)
> @@ -1405,6 +1418,19 @@ u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
>  
> +/**
> + * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits which is
> + * number of bits of precision within the decoder line buffer supported by
> + * the DSC sink. This is used to populate the DSC parameters in the
> + * &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns:
> + * Line buffer depth supported by DSC panel or 0 its invalid
> + */
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  {
>  	u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - DP_DSC_SUPPORT];
> @@ -1434,6 +1460,20 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> +/**
> + * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per component
> + * values supported by the DSC sink. This is used to populate the DSC parameters
> + * in the &struct drm_dsc_config by the driver.
> + * Driver creates an infoframe using these parameters to populate
> + * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
> + * infoframe using the helper function drm_dsc_pps_infoframe_pack()
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @dsc_bpc: An array to be filled by this helper with supported
> + *           input bpcs.
> + *
> + * Returns:
> + * Number of input BPC values parsed from the DPCD
> + */
>  int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  					 u8 dsc_bpc[3])
>  {
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index bc2b23adb072..0fd56fbdf9b4 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -17,6 +17,12 @@
>  /**
>   * DOC: dsc helpers
>   *
> + * VESA specification for DP 1.4 adds a new feature called Display Stream
> + * Compression (DSC) used to compress the pixel bits before sending it on
> + * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the existing
> + * display interfaces can support high resolutions at higher frames rates uisng
> + * the maximum available link capacity of these interfaces.
> + *
>   * These functions contain some common logic and helpers to deal with VESA
>   * Display Stream Compression standard required for DSC on Display Port/eDP or
>   * MIPI display interfaces.
> @@ -26,6 +32,7 @@
>   * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
>   * for DisplayPort as per the DP 1.4 spec.
>   * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
> + *           as defined in &struct drm_dsc_pps_infoframe
>   */
>  void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
>  {
> @@ -44,9 +51,11 @@ EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
>   * that span more than 1 byte.
>   *
>   * @pps_sdp:
> - * Secondary data packet for DSC Picture Parameter Set
> + * Secondary data packet for DSC Picture Parameter Set. This is defined
> + * by &struct drm_dsc_pps_infoframe
>   * @dsc_cfg:
> - * DSC Configuration data filled by driver
> + * DSC Configuration data filled by driver as defined by
> + * &struct drm_dsc_config
>   */
>  void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  				const struct drm_dsc_config *dsc_cfg)
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5db7fb8c8b50..2711cdfa0c13 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1052,11 +1052,18 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw);
>  #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
>  /* 0x80+ CEA-861 infoframe types */
>  
> +/**
> + * struct dp_sdp_header - DP secondary data packet header
> + * @HB0: Secondary Data Packet ID
> + * @HB1: Secondary Data Packet Type
> + * @HB2: Secondary Data Packet Specific header, Byte 0
> + * @HB3: Secondary Data packet Specific header, Byte 1
> + */
>  struct dp_sdp_header {
> -	u8 HB0; /* Secondary Data Packet ID */
> -	u8 HB1; /* Secondary Data Packet Type */
> -	u8 HB2; /* Secondary Data Packet Specific header, Byte 0 */
> -	u8 HB3; /* Secondary Data packet Specific header, Byte 1 */
> +	u8 HB0;
> +	u8 HB1;
> +	u8 HB2;
> +	u8 HB3;
>  } __packed;
>  
>  #define EDP_SDP_HEADER_REVISION_MASK		0x1F
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index d03f1b83421a..f50d265a97e2 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -44,111 +44,128 @@
>  #define DSC_1_2_MAX_LINEBUF_DEPTH_VAL		0
>  #define DSC_1_1_MAX_LINEBUF_DEPTH_BITS		13
>  
> -/* Configuration for a single Rate Control model range */
> +/**
> + * struct drm_dsc_rc_range_parameters - DSC Rate Control range parameters
> + *
> + * @range_min_qp: Min Quantization Parameters allowed for this range
> + * @range_max_qp: Max Quantization Parameters allowed for this range
> + * @range_bpg_offset: Bits/group offset to apply to target for this group
> + */
>  struct drm_dsc_rc_range_parameters {
> -	/* Min Quantization Parameters allowed for this range */
>  	u8 range_min_qp;
> -	/* Max Quantization Parameters allowed for this range */
>  	u8 range_max_qp;
> -	/* Bits/group offset to apply to target for this group */
>  	u8 range_bpg_offset;
>  };
>  
> +/**
> + * struct drm_dsc_config - Parameters required to configure DSC
> + *
> + * @line_buf_depth: Bits / component for previous reconstructed line buffer
> + * @bits_per_component: Bits per component to code (8/10/12)
> + * @convert_rgb: Flag to indicate if RGB - YCoCg conversion is needed
> + *               True if RGB input, False if YCoCg input
> + * @slice_count: Number fo slices per line used by the DSC encoder
> + * @slice_width: Width of each slice in pixels
> + * @slice_height: Slice height in pixels
> + * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
> + * @pic_width: Width of the input display frame in pixels
> + * @pic_height: Vertical height of the input display frame
> + * @rc_tgt_offset_high: Offset to bits/group used by RC to determine QP
> + *                      adjustment
> + * @rc_tgt_offset_low: Offset to bits/group used by RC to determine QP
> + *                     adjustment
> + * @bits_per_pixel: Target bits per pixel with 4 fractional bits.
> + *                  bits_per_pixel << 4
> + * @rc_edge_factor: Factor to determine if an edge is present based on the
> + *                  bits produced
> + * @rc_quant_incr_limit1: Slow down incrementing once the range reaches this
> + *                        value
> + * @rc_quant_incr_limit0: Slow down incrementing once the range reaches this
> + *                        value
> + * @initial_xmit_delay: Number of pixels to delay the initial transmission
> + * @initial_dec_delay: Initial decoder delay, number of pixel times that the
> + *                     decoer accumulates data in its rate buffer before
> + *                     starting to decode and output pixels.
> + * @block_pred_enable: True if block prediction is used to code any groups
> + *                     within the picture.
> + *                     False if BP not used
> + * @first_line_bpg_offset: Number of additional bits allocated for each group on
> + *                         the first line of slice.
> + * @initial_offset: Value to use for RC model offset at slice start
> + * @rc_buf_thresh: Thresholds defining each of the buffer ranges
> + * @rc_range_params: Parameters for each of the RC ranges defined in
> + *                   &struct drm_dsc_rc_range_parameters
> + * @rc_model_size: Total size of RC model
> + * @flatness_min_qp: Minimum QP where flatness information is sent
> + * @flatness_max_qp: Maximum QP where flatness information is sent
> + * @initial_scale_value: Initial value for the scale factor
> + * @scale_decrement_interval: Specifies number of group times between
> + *                            decrementing the scale factor at beginning
> + *                            of a slice.
> + * @scale_increment_interval: Number of group times between incrementing
> + *                            the scale factor value used at the beginning
> + *                            of a slice.
> + * @nfl_bpg_offset: Non first line BPG offset to be used
> + * @slice_bpg_offset: BPG offset used to enforce slice bit
> + * @final_offset: Final RC linear transformation offset value
> + * @vbr_enable: True if VBR mode is enabled, false if disabled
> + * @mux_word_size: Mux word size (in bits) for SSM mode
> + * @slice_chunk_size: The (max) size in bytes of the "chunks" that are
> + *                    used in slice multiplexing.
> + * @rc_bits: Rate control buffer size in bits
> + * @dsc_version_minor: DSC minor version
> + * @dsc_version_major: DSC major version
> + * @native_422: True if Native 4:2:2 supported, else false
> + * @native_420: True if Native 4:2:0 supported else false.
> + * @second_line_bpg_offset: Additional bits/grp for seconnd line of slice for
> + *                          native 4:2:0
> + * @nsl_bpg_offset: Num of bits deallocated for each grp that is not in second
> + *                  line of slice
> + * @second_line_offset_adj: Offset adjustment for second line in Native 4:2:0
> + *                          mode
> + */
>  struct drm_dsc_config {
> -	/* Bits / component for previous reconstructed line buffer */
>  	u8 line_buf_depth;
> -	/* Bits per component to code (must be 8, 10, or 12) */
>  	u8 bits_per_component;
> -	/*
> -	 * Flag indicating to do RGB - YCoCg conversion
> -	 * and back (should be 1 for RGB input)
> -	 */
>  	bool convert_rgb;
>  	u8 slice_count;
> -	/* Slice Width */
>  	u16 slice_width;
> -	/* Slice Height */
>  	u16 slice_height;
> -	/*
> -	 * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
> -	 * outside of DSC encode/decode algorithm)
> -	 */
>  	bool enable422;
> -	/* Picture Width */
>  	u16 pic_width;
> -	/* Picture Height */
>  	u16 pic_height;
> -	/* Offset to bits/group used by RC to determine QP adjustment */
>  	u8 rc_tgt_offset_high;
> -	/* Offset to bits/group used by RC to determine QP adjustment */
>  	u8 rc_tgt_offset_low;
> -	/* Bits/pixel target << 4 (ie., 4 fractional bits) */
>  	u16 bits_per_pixel;
> -	/*
> -	 * Factor to determine if an edge is present based
> -	 * on the bits produced
> -	 */
>  	u8 rc_edge_factor;
> -	/* Slow down incrementing once the range reaches this value */
>  	u8 rc_quant_incr_limit1;
> -	/* Slow down incrementing once the range reaches this value */
>  	u8 rc_quant_incr_limit0;
> -	/* Number of pixels to delay the initial transmission */
>  	u16 initial_xmit_delay;
> -	/* Number of pixels to delay the VLD on the decoder,not including SSM */
>  	u16  initial_dec_delay;
> -	/* Block prediction enable */
>  	bool block_pred_enable;
> -	/* Bits/group offset to use for first line of the slice */
>  	u8 first_line_bpg_offset;
> -	/* Value to use for RC model offset at slice start */
>  	u16 initial_offset;
> -	/* Thresholds defining each of the buffer ranges */
>  	u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1];
> -	/* Parameters for each of the RC ranges */
>  	struct drm_dsc_rc_range_parameters rc_range_params[DSC_NUM_BUF_RANGES];
> -	/* Total size of RC model */
>  	u16 rc_model_size;
> -	/* Minimum QP where flatness information is sent */
>  	u8 flatness_min_qp;
> -	/* Maximum QP where flatness information is sent */
>  	u8 flatness_max_qp;
> -	/* Initial value for scale factor */
>  	u8 initial_scale_value;
> -	/* Decrement scale factor every scale_decrement_interval groups */
>  	u16 scale_decrement_interval;
> -	/* Increment scale factor every scale_increment_interval groups */
>  	u16 scale_increment_interval;
> -	/* Non-first line BPG offset to use */
>  	u16 nfl_bpg_offset;
> -	/* BPG offset used to enforce slice bit */
>  	u16 slice_bpg_offset;
> -	/* Final RC linear transformation offset value */
>  	u16 final_offset;
> -	/* Enable on-off VBR (ie., disable stuffing bits) */
>  	bool vbr_enable;
> -	/* Mux word size (in bits) for SSM mode */
>  	u8 mux_word_size;
> -	/*
> -	 * The (max) size in bytes of the "chunks" that are
> -	 * used in slice multiplexing
> -	 */
>  	u16 slice_chunk_size;
> -	/* Rate Control buffer siz in bits */
>  	u16 rc_bits;
> -	/* DSC Minor Version */
>  	u8 dsc_version_minor;
> -	/* DSC Major version */
>  	u8 dsc_version_major;
> -	/* Native 4:2:2 support */
>  	bool native_422;
> -	/* Native 4:2:0 support */
>  	bool native_420;
> -	/* Additional bits/grp for seconnd line of slice for native 4:2:0 */
>  	u8 second_line_bpg_offset;
> -	/* Num of bits deallocated for each grp that is not in second line of slice */
>  	u16 nsl_bpg_offset;
> -	/* Offset adj fr second line in Native 4:2:0 mode */
>  	u16 second_line_offset_adj;
>  };
>  
> @@ -468,10 +485,13 @@ struct drm_dsc_picture_parameter_set {
>   * This structure represents the DSC PPS infoframe required to send the Picture
>   * Parameter Set metadata required before enabling VESA Display Stream
>   * Compression. This is based on the DP Secondary Data Packet structure and
> - * comprises of SDP Header as defined in drm_dp_helper.h and PPS payload.
> + * comprises of SDP Header as defined &struct struct dp_sdp_header in drm_dp_helper.h
> + * and PPS payload defined in &struct drm_dsc_picture_parameter_set.
>   *
> - * @pps_header: Header for PPS as per DP SDP header format
> + * @pps_header: Header for PPS as per DP SDP header format of type
> + *              &struct dp_sdp_header
>   * @pps_payload: PPS payload fields as per DSC specification Table 4-1
> + *               as represented in &struct drm_dsc_picture_parameter_set
>   */
>  struct drm_dsc_pps_infoframe {
>  	struct dp_sdp_header pps_header;
> -- 
> 2.19.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 54120b6319e7..e9e0233f5b2f 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1360,7 +1360,20 @@  int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
 EXPORT_SYMBOL(drm_dp_read_desc);
 
 /**
- * DRM DP Helpers for DSC
+ * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
+ * supported by the DSC sink.
+ * @dsc_dpcd: DSC capabilities from DPCD
+ * @is_edp: true if its eDP, false for DP
+ *
+ * Read the slice capabilities DPCD register from DSC sink to get
+ * the maximum slice count supported. This is used to populate
+ * the DSC parameters in the &struct drm_dsc_config by the driver.
+ * Driver creates an infoframe using these parameters to populate
+ * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
+ * infoframe using the helper function drm_dsc_pps_infoframe_pack()
+ *
+ * Returns:
+ * Maximum slice count supported by DSC sink or 0 its invalid
  */
 u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
 				   bool is_edp)
@@ -1405,6 +1418,19 @@  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
 }
 EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
 
+/**
+ * drm_dp_dsc_sink_line_buf_depth() - Get the line buffer depth in bits which is
+ * number of bits of precision within the decoder line buffer supported by
+ * the DSC sink. This is used to populate the DSC parameters in the
+ * &struct drm_dsc_config by the driver.
+ * Driver creates an infoframe using these parameters to populate
+ * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
+ * infoframe using the helper function drm_dsc_pps_infoframe_pack()
+ * @dsc_dpcd: DSC capabilities from DPCD
+ *
+ * Returns:
+ * Line buffer depth supported by DSC panel or 0 its invalid
+ */
 u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
 {
 	u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - DP_DSC_SUPPORT];
@@ -1434,6 +1460,20 @@  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
 }
 EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
 
+/**
+ * drm_dp_dsc_sink_supported_input_bpcs() - Get all the input bits per component
+ * values supported by the DSC sink. This is used to populate the DSC parameters
+ * in the &struct drm_dsc_config by the driver.
+ * Driver creates an infoframe using these parameters to populate
+ * &struct drm_dsc_pps_infoframe. These are sent to the sink using DSC
+ * infoframe using the helper function drm_dsc_pps_infoframe_pack()
+ * @dsc_dpcd: DSC capabilities from DPCD
+ * @dsc_bpc: An array to be filled by this helper with supported
+ *           input bpcs.
+ *
+ * Returns:
+ * Number of input BPC values parsed from the DPCD
+ */
 int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
 					 u8 dsc_bpc[3])
 {
diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index bc2b23adb072..0fd56fbdf9b4 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -17,6 +17,12 @@ 
 /**
  * DOC: dsc helpers
  *
+ * VESA specification for DP 1.4 adds a new feature called Display Stream
+ * Compression (DSC) used to compress the pixel bits before sending it on
+ * DP/eDP/MIPI DSI interface. DSC is required to be enabled so that the existing
+ * display interfaces can support high resolutions at higher frames rates uisng
+ * the maximum available link capacity of these interfaces.
+ *
  * These functions contain some common logic and helpers to deal with VESA
  * Display Stream Compression standard required for DSC on Display Port/eDP or
  * MIPI display interfaces.
@@ -26,6 +32,7 @@ 
  * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
  * for DisplayPort as per the DP 1.4 spec.
  * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
+ *           as defined in &struct drm_dsc_pps_infoframe
  */
 void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
 {
@@ -44,9 +51,11 @@  EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
  * that span more than 1 byte.
  *
  * @pps_sdp:
- * Secondary data packet for DSC Picture Parameter Set
+ * Secondary data packet for DSC Picture Parameter Set. This is defined
+ * by &struct drm_dsc_pps_infoframe
  * @dsc_cfg:
- * DSC Configuration data filled by driver
+ * DSC Configuration data filled by driver as defined by
+ * &struct drm_dsc_config
  */
 void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 				const struct drm_dsc_config *dsc_cfg)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5db7fb8c8b50..2711cdfa0c13 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1052,11 +1052,18 @@  int drm_dp_bw_code_to_link_rate(u8 link_bw);
 #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
 /* 0x80+ CEA-861 infoframe types */
 
+/**
+ * struct dp_sdp_header - DP secondary data packet header
+ * @HB0: Secondary Data Packet ID
+ * @HB1: Secondary Data Packet Type
+ * @HB2: Secondary Data Packet Specific header, Byte 0
+ * @HB3: Secondary Data packet Specific header, Byte 1
+ */
 struct dp_sdp_header {
-	u8 HB0; /* Secondary Data Packet ID */
-	u8 HB1; /* Secondary Data Packet Type */
-	u8 HB2; /* Secondary Data Packet Specific header, Byte 0 */
-	u8 HB3; /* Secondary Data packet Specific header, Byte 1 */
+	u8 HB0;
+	u8 HB1;
+	u8 HB2;
+	u8 HB3;
 } __packed;
 
 #define EDP_SDP_HEADER_REVISION_MASK		0x1F
diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
index d03f1b83421a..f50d265a97e2 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -44,111 +44,128 @@ 
 #define DSC_1_2_MAX_LINEBUF_DEPTH_VAL		0
 #define DSC_1_1_MAX_LINEBUF_DEPTH_BITS		13
 
-/* Configuration for a single Rate Control model range */
+/**
+ * struct drm_dsc_rc_range_parameters - DSC Rate Control range parameters
+ *
+ * @range_min_qp: Min Quantization Parameters allowed for this range
+ * @range_max_qp: Max Quantization Parameters allowed for this range
+ * @range_bpg_offset: Bits/group offset to apply to target for this group
+ */
 struct drm_dsc_rc_range_parameters {
-	/* Min Quantization Parameters allowed for this range */
 	u8 range_min_qp;
-	/* Max Quantization Parameters allowed for this range */
 	u8 range_max_qp;
-	/* Bits/group offset to apply to target for this group */
 	u8 range_bpg_offset;
 };
 
+/**
+ * struct drm_dsc_config - Parameters required to configure DSC
+ *
+ * @line_buf_depth: Bits / component for previous reconstructed line buffer
+ * @bits_per_component: Bits per component to code (8/10/12)
+ * @convert_rgb: Flag to indicate if RGB - YCoCg conversion is needed
+ *               True if RGB input, False if YCoCg input
+ * @slice_count: Number fo slices per line used by the DSC encoder
+ * @slice_width: Width of each slice in pixels
+ * @slice_height: Slice height in pixels
+ * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
+ * @pic_width: Width of the input display frame in pixels
+ * @pic_height: Vertical height of the input display frame
+ * @rc_tgt_offset_high: Offset to bits/group used by RC to determine QP
+ *                      adjustment
+ * @rc_tgt_offset_low: Offset to bits/group used by RC to determine QP
+ *                     adjustment
+ * @bits_per_pixel: Target bits per pixel with 4 fractional bits.
+ *                  bits_per_pixel << 4
+ * @rc_edge_factor: Factor to determine if an edge is present based on the
+ *                  bits produced
+ * @rc_quant_incr_limit1: Slow down incrementing once the range reaches this
+ *                        value
+ * @rc_quant_incr_limit0: Slow down incrementing once the range reaches this
+ *                        value
+ * @initial_xmit_delay: Number of pixels to delay the initial transmission
+ * @initial_dec_delay: Initial decoder delay, number of pixel times that the
+ *                     decoer accumulates data in its rate buffer before
+ *                     starting to decode and output pixels.
+ * @block_pred_enable: True if block prediction is used to code any groups
+ *                     within the picture.
+ *                     False if BP not used
+ * @first_line_bpg_offset: Number of additional bits allocated for each group on
+ *                         the first line of slice.
+ * @initial_offset: Value to use for RC model offset at slice start
+ * @rc_buf_thresh: Thresholds defining each of the buffer ranges
+ * @rc_range_params: Parameters for each of the RC ranges defined in
+ *                   &struct drm_dsc_rc_range_parameters
+ * @rc_model_size: Total size of RC model
+ * @flatness_min_qp: Minimum QP where flatness information is sent
+ * @flatness_max_qp: Maximum QP where flatness information is sent
+ * @initial_scale_value: Initial value for the scale factor
+ * @scale_decrement_interval: Specifies number of group times between
+ *                            decrementing the scale factor at beginning
+ *                            of a slice.
+ * @scale_increment_interval: Number of group times between incrementing
+ *                            the scale factor value used at the beginning
+ *                            of a slice.
+ * @nfl_bpg_offset: Non first line BPG offset to be used
+ * @slice_bpg_offset: BPG offset used to enforce slice bit
+ * @final_offset: Final RC linear transformation offset value
+ * @vbr_enable: True if VBR mode is enabled, false if disabled
+ * @mux_word_size: Mux word size (in bits) for SSM mode
+ * @slice_chunk_size: The (max) size in bytes of the "chunks" that are
+ *                    used in slice multiplexing.
+ * @rc_bits: Rate control buffer size in bits
+ * @dsc_version_minor: DSC minor version
+ * @dsc_version_major: DSC major version
+ * @native_422: True if Native 4:2:2 supported, else false
+ * @native_420: True if Native 4:2:0 supported else false.
+ * @second_line_bpg_offset: Additional bits/grp for seconnd line of slice for
+ *                          native 4:2:0
+ * @nsl_bpg_offset: Num of bits deallocated for each grp that is not in second
+ *                  line of slice
+ * @second_line_offset_adj: Offset adjustment for second line in Native 4:2:0
+ *                          mode
+ */
 struct drm_dsc_config {
-	/* Bits / component for previous reconstructed line buffer */
 	u8 line_buf_depth;
-	/* Bits per component to code (must be 8, 10, or 12) */
 	u8 bits_per_component;
-	/*
-	 * Flag indicating to do RGB - YCoCg conversion
-	 * and back (should be 1 for RGB input)
-	 */
 	bool convert_rgb;
 	u8 slice_count;
-	/* Slice Width */
 	u16 slice_width;
-	/* Slice Height */
 	u16 slice_height;
-	/*
-	 * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
-	 * outside of DSC encode/decode algorithm)
-	 */
 	bool enable422;
-	/* Picture Width */
 	u16 pic_width;
-	/* Picture Height */
 	u16 pic_height;
-	/* Offset to bits/group used by RC to determine QP adjustment */
 	u8 rc_tgt_offset_high;
-	/* Offset to bits/group used by RC to determine QP adjustment */
 	u8 rc_tgt_offset_low;
-	/* Bits/pixel target << 4 (ie., 4 fractional bits) */
 	u16 bits_per_pixel;
-	/*
-	 * Factor to determine if an edge is present based
-	 * on the bits produced
-	 */
 	u8 rc_edge_factor;
-	/* Slow down incrementing once the range reaches this value */
 	u8 rc_quant_incr_limit1;
-	/* Slow down incrementing once the range reaches this value */
 	u8 rc_quant_incr_limit0;
-	/* Number of pixels to delay the initial transmission */
 	u16 initial_xmit_delay;
-	/* Number of pixels to delay the VLD on the decoder,not including SSM */
 	u16  initial_dec_delay;
-	/* Block prediction enable */
 	bool block_pred_enable;
-	/* Bits/group offset to use for first line of the slice */
 	u8 first_line_bpg_offset;
-	/* Value to use for RC model offset at slice start */
 	u16 initial_offset;
-	/* Thresholds defining each of the buffer ranges */
 	u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1];
-	/* Parameters for each of the RC ranges */
 	struct drm_dsc_rc_range_parameters rc_range_params[DSC_NUM_BUF_RANGES];
-	/* Total size of RC model */
 	u16 rc_model_size;
-	/* Minimum QP where flatness information is sent */
 	u8 flatness_min_qp;
-	/* Maximum QP where flatness information is sent */
 	u8 flatness_max_qp;
-	/* Initial value for scale factor */
 	u8 initial_scale_value;
-	/* Decrement scale factor every scale_decrement_interval groups */
 	u16 scale_decrement_interval;
-	/* Increment scale factor every scale_increment_interval groups */
 	u16 scale_increment_interval;
-	/* Non-first line BPG offset to use */
 	u16 nfl_bpg_offset;
-	/* BPG offset used to enforce slice bit */
 	u16 slice_bpg_offset;
-	/* Final RC linear transformation offset value */
 	u16 final_offset;
-	/* Enable on-off VBR (ie., disable stuffing bits) */
 	bool vbr_enable;
-	/* Mux word size (in bits) for SSM mode */
 	u8 mux_word_size;
-	/*
-	 * The (max) size in bytes of the "chunks" that are
-	 * used in slice multiplexing
-	 */
 	u16 slice_chunk_size;
-	/* Rate Control buffer siz in bits */
 	u16 rc_bits;
-	/* DSC Minor Version */
 	u8 dsc_version_minor;
-	/* DSC Major version */
 	u8 dsc_version_major;
-	/* Native 4:2:2 support */
 	bool native_422;
-	/* Native 4:2:0 support */
 	bool native_420;
-	/* Additional bits/grp for seconnd line of slice for native 4:2:0 */
 	u8 second_line_bpg_offset;
-	/* Num of bits deallocated for each grp that is not in second line of slice */
 	u16 nsl_bpg_offset;
-	/* Offset adj fr second line in Native 4:2:0 mode */
 	u16 second_line_offset_adj;
 };
 
@@ -468,10 +485,13 @@  struct drm_dsc_picture_parameter_set {
  * This structure represents the DSC PPS infoframe required to send the Picture
  * Parameter Set metadata required before enabling VESA Display Stream
  * Compression. This is based on the DP Secondary Data Packet structure and
- * comprises of SDP Header as defined in drm_dp_helper.h and PPS payload.
+ * comprises of SDP Header as defined &struct struct dp_sdp_header in drm_dp_helper.h
+ * and PPS payload defined in &struct drm_dsc_picture_parameter_set.
  *
- * @pps_header: Header for PPS as per DP SDP header format
+ * @pps_header: Header for PPS as per DP SDP header format of type
+ *              &struct dp_sdp_header
  * @pps_payload: PPS payload fields as per DSC specification Table 4-1
+ *               as represented in &struct drm_dsc_picture_parameter_set
  */
 struct drm_dsc_pps_infoframe {
 	struct dp_sdp_header pps_header;