Pushed to drm-misc thanks for the reviews.

Regards
Manasi

On Wed, Feb 06, 2019 at 01:31:48PM -0800, Manasi Navare wrote:
> This patch adds appropriate 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.
> 
> v3:
> * Fix the checkpatch warnings (Sean Paul)
> v2:
> * Add inline comments for longer structs (Daniel Vetter)
> * Split the summary and description (Daniel Vetter)
> 
> Suggested-by: Daniel Vetter <daniel.vet...@intel.com>
> Suggested-by: Sean Paul <s...@poorly.run>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Sean Paul <s...@poorly.run>
> Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com>
> Acked-by: Sean Paul <s...@poorly.run>
> Reviewed-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c |  47 ++++++-
>  drivers/gpu/drm/drm_dsc.c       |  30 +++-
>  include/drm/drm_dp_helper.h     |  15 +-
>  include/drm/drm_dsc.h           | 233 ++++++++++++++++++++++++--------
>  4 files changed, 259 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 54120b6319e7..54a6414c5d96 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,21 @@ 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
> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Read the DSC DPCD register to parse 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()
> + *
> + * 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 +1462,23 @@ 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.
> + * @dsc_dpcd: DSC capabilities from DPCD
> + * @dsc_bpc: An array to be filled by this helper with supported
> + *           input bpcs.
> + *
> + * Read the DSC DPCD from the sink device to parse the supported bits per
> + * component values. 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:
> + * 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..bce99f95c1a3 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,13 @@
>   * 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
> + *
> + * DP 1.4 spec defines the secondary data packet for sending the
> + * picture parameter infoframes from the source to the sink.
> + * This function populates the pps header defined in
> + * &struct drm_dsc_pps_infoframe as per the header bytes defined
> + * in &struct dp_sdp_header.
>   */
>  void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
>  {
> @@ -38,15 +51,20 @@ EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
>  
>  /**
>   * drm_dsc_pps_infoframe_pack() - Populates the DSC PPS infoframe
> - * using the DSC configuration parameters in the order expected
> - * by the DSC Display Sink device. For the DSC, the sink device
> - * expects the PPS payload in the big endian format for the fields
> - * 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
> + *
> + * DSC source device sends a secondary data packet filled with all the
> + * picture parameter set (PPS) information required by the sink to decode
> + * the compressed frame. Driver populates the dsC PPS infoframe using the DSC
> + * configuration parameters in the order expected by the DSC Display Sink
> + * device. For the DSC, the sink device expects the PPS payload in the big
> + * endian format for the fields that span more than 1 byte.
>   */
>  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..9c26f083c70f 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -44,111 +44,231 @@
>  #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
> + *
> + * This defines different rate control parameters used by the DSC engine
> + * to compress the frame.
> + */
>  struct drm_dsc_rc_range_parameters {
> -     /* Min Quantization Parameters allowed for this range */
> +     /**
> +      * @range_min_qp: Min Quantization Parameters allowed for this range
> +      */
>       u8 range_min_qp;
> -     /* Max Quantization Parameters allowed for this range */
> +     /**
> +      * @range_max_qp: Max Quantization Parameters allowed for this range
> +      */
>       u8 range_max_qp;
> -     /* Bits/group offset to apply to target for this group */
> +     /**
> +      * @range_bpg_offset:
> +      * Bits/group offset to apply to target for this group
> +      */
>       u8 range_bpg_offset;
>  };
>  
> +/**
> + * struct drm_dsc_config - Parameters required to configure DSC
> + *
> + * Driver populates this structure with all the parameters required
> + * to configure the display stream compression on the source.
> + */
>  struct drm_dsc_config {
> -     /* Bits / component for previous reconstructed line buffer */
> +     /**
> +      * @line_buf_depth:
> +      * Bits per component for previous reconstructed line buffer
> +      */
>       u8 line_buf_depth;
> -     /* Bits per component to code (must be 8, 10, or 12) */
> +     /**
> +      * @bits_per_component: Bits per component to code (8/10/12)
> +      */
>       u8 bits_per_component;
> -     /*
> -      * Flag indicating to do RGB - YCoCg conversion
> -      * and back (should be 1 for RGB input)
> +     /**
> +      * @convert_rgb:
> +      * Flag to indicate if RGB - YCoCg conversion is needed
> +      * True if RGB input, False if YCoCg input
>        */
>       bool convert_rgb;
> +     /**
> +      * @slice_count: Number fo slices per line used by the DSC encoder
> +      */
>       u8 slice_count;
> -     /* Slice Width */
> +     /**
> +      *  @slice_width: Width of each slice in pixels
> +      */
>       u16 slice_width;
> -     /* Slice Height */
> +     /**
> +      * @slice_height: Slice height in pixels
> +      */
>       u16 slice_height;
> -     /*
> -      * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
> -      * outside of DSC encode/decode algorithm)
> +     /**
> +      * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
>        */
>       bool enable422;
> -     /* Picture Width */
> +     /**
> +      * @pic_width: Width of the input display frame in pixels
> +      */
>       u16 pic_width;
> -     /* Picture Height */
> +     /**
> +      * @pic_height: Vertical height of the input display frame
> +      */
>       u16 pic_height;
> -     /* Offset to bits/group used by RC to determine QP adjustment */
> +     /**
> +      * @rc_tgt_offset_high:
> +      * 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 */
> +     /**
> +      * @rc_tgt_offset_low:
> +      * Offset to bits/group used by RC to determine QP adjustment
> +      */
>       u8 rc_tgt_offset_low;
> -     /* Bits/pixel target << 4 (ie., 4 fractional bits) */
> +     /**
> +      * @bits_per_pixel:
> +      * Target bits per pixel with 4 fractional bits, bits_per_pixel << 4
> +      */
>       u16 bits_per_pixel;
> -     /*
> -      * Factor to determine if an edge is present based
> -      * on the bits produced
> +     /**
> +      * @rc_edge_factor:
> +      * 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 */
> +     /**
> +      * @rc_quant_incr_limit1:
> +      * Slow down incrementing once the range reaches this value
> +      */
>       u8 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
> +      */
>       u8 rc_quant_incr_limit0;
> -     /* Number of pixels to delay the initial transmission */
> +     /**
> +      * @initial_xmit_delay:
> +      * 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 */
> +     /**
> +      * @initial_dec_delay:
> +      * Initial decoder delay, number of pixel times that the decoder
> +      * accumulates data in its rate buffer before starting to decode
> +      * and output pixels.
> +      */
>       u16  initial_dec_delay;
> -     /* Block prediction enable */
> +     /**
> +      * @block_pred_enable:
> +      * True if block prediction is used to code any groups within the
> +      * picture. False if BP not used
> +      */
>       bool block_pred_enable;
> -     /* Bits/group offset to use for first line of the slice */
> +     /**
> +      * @first_line_bpg_offset:
> +      * Number of additional bits allocated for each group on the first
> +      * line of slice.
> +      */
>       u8 first_line_bpg_offset;
> -     /* Value to use for RC model offset at slice start */
> +     /**
> +      * @initial_offset: Value to use for RC model offset at slice start
> +      */
>       u16 initial_offset;
> -     /* Thresholds defining each of the buffer ranges */
> +     /**
> +      * @rc_buf_thresh: Thresholds defining each of the buffer ranges
> +      */
>       u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1];
> -     /* Parameters for each of the RC ranges */
> +     /**
> +      * @rc_range_params:
> +      * Parameters for each of the RC ranges defined in
> +      * &struct drm_dsc_rc_range_parameters
> +      */
>       struct drm_dsc_rc_range_parameters rc_range_params[DSC_NUM_BUF_RANGES];
> -     /* Total size of RC model */
> +     /**
> +      * @rc_model_size: Total size of RC model
> +      */
>       u16 rc_model_size;
> -     /* Minimum QP where flatness information is sent */
> +     /**
> +      * @flatness_min_qp: Minimum QP where flatness information is sent
> +      */
>       u8 flatness_min_qp;
> -     /* Maximum QP where flatness information is sent */
> +     /**
> +      * @flatness_max_qp: Maximum QP where flatness information is sent
> +      */
>       u8 flatness_max_qp;
> -     /* Initial value for scale factor */
> +     /**
> +      * @initial_scale_value: Initial value for the scale factor
> +      */
>       u8 initial_scale_value;
> -     /* Decrement scale factor every scale_decrement_interval groups */
> +     /**
> +      * @scale_decrement_interval:
> +      * Specifies number of group times between decrementing the scale factor
> +      * at beginning of a slice.
> +      */
>       u16 scale_decrement_interval;
> -     /* Increment scale factor every scale_increment_interval groups */
> +     /**
> +      * @scale_increment_interval:
> +      * Number of group times between incrementing the scale factor value
> +      * used at the beginning of a slice.
> +      */
>       u16 scale_increment_interval;
> -     /* Non-first line BPG offset to use */
> +     /**
> +      * @nfl_bpg_offset: Non first line BPG offset to be used
> +      */
>       u16 nfl_bpg_offset;
> -     /* BPG offset used to enforce slice bit */
> +     /**
> +      * @slice_bpg_offset: BPG offset used to enforce slice bit
> +      */
>       u16 slice_bpg_offset;
> -     /* Final RC linear transformation offset value */
> +     /**
> +      * @final_offset: Final RC linear transformation offset value
> +      */
>       u16 final_offset;
> -     /* Enable on-off VBR (ie., disable stuffing bits) */
> +     /**
> +      * @vbr_enable: True if VBR mode is enabled, false if disabled
> +      */
>       bool vbr_enable;
> -     /* Mux word size (in bits) for SSM mode */
> +     /**
> +      * @mux_word_size: 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
> +     /**
> +      * @slice_chunk_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 */
> +     /**
> +      * @rc_bits: Rate control buffer size in bits
> +      */
>       u16 rc_bits;
> -     /* DSC Minor Version */
> +     /**
> +      * @dsc_version_minor: DSC minor version
> +      */
>       u8 dsc_version_minor;
> -     /* DSC Major version */
> +     /**
> +      * @dsc_version_major: DSC major version
> +      */
>       u8 dsc_version_major;
> -     /* Native 4:2:2 support */
> +     /**
> +      * @native_422: True if Native 4:2:2 supported, else false
> +      */
>       bool native_422;
> -     /* Native 4:2:0 support */
> +     /**
> +      * @native_420: True if Native 4:2:0 supported else false.
> +      */
>       bool native_420;
> -     /* Additional bits/grp for seconnd line of slice for native 4:2:0 */
> +     /**
> +      * @second_line_bpg_offset:
> +      * 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 */
> +     /**
> +      * @nsl_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 */
> +     /**
> +      * @second_line_offset_adj:
> +      * Offset adjustment for second line in Native 4:2:0 mode
> +      */
>       u16 second_line_offset_adj;
>  };
>  
> @@ -468,10 +588,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

Reply via email to