Re: [PATCH v6 11/28] drm/dsc: Add helpers for DSC picture parameter set infoframes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 24, 2018 at 03:28:23PM -0700, Manasi Navare wrote:
> According to Display Stream compression spec 1.2, the picture
> parameter set metadata is sent from source to sink device
> using the DP Secondary data packet. An infoframe is formed
> for the PPS SDP header and PPS SDP payload bytes.
> This patch adds helpers to fill the PPS SDP header
> and PPS SDP payload according to the DSC 1.2 specification.
> 
> v6:
> * Use proper sequence points for breaking down the
> assignments (Chris Wilson)
> * Use SPDX identifier
> v5:
> Do not use bitfields for DRM structs (Jani N)
> v4:
> * Use DSC constants for params that dont change across
> configurations
> v3:
> * Add reference to added kernel-docs in Documentation/gpu/drm-kms-helpers.rst
> (Daniel Vetter)
> 
> v2:
> * Add EXPORT_SYMBOL for the drm functions (Manasi)
> 
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> Acked-by: Harry Wentland <harry.wentland@xxxxxxx>
> ---
>  Documentation/gpu/drm-kms-helpers.rst |  12 ++
>  drivers/gpu/drm/Makefile              |   2 +-
>  drivers/gpu/drm/drm_dsc.c             | 223 ++++++++++++++++++++++++++
>  include/drm/drm_dsc.h                 |  22 +++
>  4 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_dsc.c
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index 4b4dc236ef6f..b422eb8edf16 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -232,6 +232,18 @@ MIPI DSI Helper Functions Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_mipi_dsi.c
>     :export:
>  
> +Display Stream Compression Helper Functions Reference
> +=====================================================
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_dsc.c
> +   :doc: dsc helpers
> +
> +.. kernel-doc:: include/drm/drm_dsc.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_dsc.c
> +   :export:
> +
>  Output Probing Helper Functions Reference
>  =========================================
>  
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 576ba985e138..3a3e6fb6d476 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,7 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>  drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
>  drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>  
> -drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> +drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper.o \
>  		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
>  		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
>  		drm_simple_kms_helper.o drm_modeset_helper.o \
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> new file mode 100644
> index 000000000000..21ae8d015afd
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2018 Intel Corp
> + *
> + * Author:
> + * Manasi Navare <manasi.d.navare@xxxxxxxxx>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <drm/drm_dp_helper.h>
> +#include <drm/drm_dsc.h>
> +
> +/**
> + * DOC: dsc helpers
> + *
> + * 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.
> + */
> +
> +/**
> + * 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
> + */
> +void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
> +{
> +	memset(&pps_sdp->pps_header, 0, sizeof(pps_sdp->pps_header));
> +
> +	pps_sdp->pps_header.HB1 = DP_SDP_PPS;
> +	pps_sdp->pps_header.HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
> +}
> +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
> + * @dsc_cfg:
> + * DSC Configuration data filled by driver
> + */
> +void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> +				struct drm_dsc_config *dsc_cfg)
> +{
> +	u8 i = 0;

int i;

> +
> +	memset(&pps_sdp->pps_payload, 0, sizeof(pps_sdp->pps_payload));
> +
> +	/* PPS 0 */
> +	pps_sdp->pps_payload.dsc_version =
> +		dsc_cfg->dsc_version_minor |
> +		dsc_cfg->dsc_version_major << DSC_PPS_VERSION_MAJOR_SHIFT;
> +
> +	/* PPS 1, 2 is 0 */
> +
> +	/* PPS 3 */
> +	pps_sdp->pps_payload.pps_3 =
> +		dsc_cfg->line_buf_depth |
> +		dsc_cfg->bits_per_component << DSC_PPS_BPC_SHIFT;
> +
> +	/* PPS 4 */
> +	pps_sdp->pps_payload.pps_4 =
> +		(u8)((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
> +		     DSC_PPS_MSB_SHIFT) |
> +		(u8)dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
> +		(u8)dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
> +		(u8)dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
> +		(u8)dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;

What's with all the (u8) casts?

> +
> +	/* PPS 5 */
> +	pps_sdp->pps_payload.bits_per_pixel_low =
> +		(u8)(dsc_cfg->bits_per_pixel & DSC_PPS_LSB_MASK);
> +
> +	/*
> +	 * The DSC panel expects the PPS packet to have big endian format
> +	 * for data spanning 2 bytes. Use a macro cpu_to_be16() to convert
> +	 * to big endian format. If format is little endian, it will swap
> +	 * bytes to convert to Big endian else keep it unchanged.
> +	 */
> +
> +	/* PPS 6, 7 */
> +	pps_sdp->pps_payload.pic_height = cpu_to_be16(dsc_cfg->pic_height);
> +
> +	/* PPS 8, 9 */
> +	pps_sdp->pps_payload.pic_width = cpu_to_be16(dsc_cfg->pic_width);
> +
> +	/* PPS 10, 11 */
> +	pps_sdp->pps_payload.slice_height = cpu_to_be16(dsc_cfg->slice_height);
> +
> +	/* PPS 12, 13 */
> +	pps_sdp->pps_payload.slice_width = cpu_to_be16(dsc_cfg->slice_width);
> +
> +	/* PPS 14, 15 */
> +	pps_sdp->pps_payload.chunk_size = cpu_to_be16(dsc_cfg->slice_chunk_size);
> +
> +	/* PPS 16 */
> +	pps_sdp->pps_payload.initial_xmit_delay_high =
> +		(u8)((dsc_cfg->initial_xmit_delay &
> +		      DSC_PPS_INIT_XMIT_DELAY_HIGH_MASK) >>
> +		     DSC_PPS_MSB_SHIFT);
> +
> +	/* PPS 17 */
> +	pps_sdp->pps_payload.initial_xmit_delay_low =
> +		(u8)(dsc_cfg->initial_xmit_delay & DSC_PPS_LSB_MASK);
> +
> +	/* PPS 18, 19 */
> +	pps_sdp->pps_payload.initial_dec_delay =
> +		cpu_to_be16(dsc_cfg->initial_dec_delay);
> +
> +	/* PPS 20 is 0 */
> +
> +	/* PPS 21 */
> +	pps_sdp->pps_payload.initial_scale_value =
> +		(u8)dsc_cfg->initial_scale_value;
> +
> +	/* PPS 22, 23 */
> +	pps_sdp->pps_payload.scale_increment_interval =
> +		cpu_to_be16(dsc_cfg->scale_increment_interval);
> +
> +	/* PPS 24 */
> +	pps_sdp->pps_payload.scale_decrement_interval_high =
> +		(u8)((dsc_cfg->scale_decrement_interval &
> +		      DSC_PPS_SCALE_DEC_INT_HIGH_MASK) >>
> +		     DSC_PPS_MSB_SHIFT);
> +
> +	/* PPS 25 */
> +	pps_sdp->pps_payload.scale_decrement_interval_low =
> +		(u8)(dsc_cfg->scale_decrement_interval & DSC_PPS_LSB_MASK);
> +
> +	/* PPS 26[7:0], PPS 27[7:5] RESERVED */
> +
> +	/* PPS 27 */
> +	pps_sdp->pps_payload.first_line_bpg_offset =
> +		(u8)dsc_cfg->first_line_bpg_offset;
> +
> +	/* PPS 28, 29 */
> +	pps_sdp->pps_payload.nfl_bpg_offset =
> +		cpu_to_be16(dsc_cfg->nfl_bpg_offset);
> +
> +	/* PPS 30, 31 */
> +	pps_sdp->pps_payload.slice_bpg_offset =
> +		cpu_to_be16(dsc_cfg->slice_bpg_offset);
> +
> +	/* PPS 32, 33 */
> +	pps_sdp->pps_payload.initial_offset =
> +		cpu_to_be16(dsc_cfg->initial_offset);
> +
> +	/* PPS 34, 35 */
> +	pps_sdp->pps_payload.final_offset = cpu_to_be16(dsc_cfg->final_offset);
> +
> +	/* PPS 36 */
> +	pps_sdp->pps_payload.flatness_min_qp = (u8)dsc_cfg->flatness_min_qp;
> +
> +	/* PPS 37 */
> +	pps_sdp->pps_payload.flatness_max_qp = (u8)dsc_cfg->flatness_max_qp;
> +
> +	/* PPS 38, 39 */
> +	pps_sdp->pps_payload.rc_model_size =
> +		cpu_to_be16((u16)DSC_RC_MODEL_SIZE_CONST);
> +
> +	/* PPS 40 */
> +	pps_sdp->pps_payload.rc_edge_factor = (u8)DSC_RC_EDGE_FACTOR_CONST;
> +
> +	/* PPS 41 */
> +	pps_sdp->pps_payload.rc_quant_incr_limit0 =
> +		(u8)dsc_cfg->rc_quant_incr_limit0;
> +
> +	/* PPS 42 */
> +	pps_sdp->pps_payload.rc_quant_incr_limit1 =
> +		(u8)dsc_cfg->rc_quant_incr_limit1;
> +
> +	/* PPS 43 */
> +	pps_sdp->pps_payload.rc_tgt_offset = (u8)DSC_RC_TGT_OFFSET_LO_CONST |
> +		(u8)DSC_RC_TGT_OFFSET_HI_CONST << DSC_PPS_RC_TGT_OFFSET_HI_SHIFT;
> +
> +	/* PPS 44 - 57 */
> +	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++)

Is the -1 correct? The loop below doesn't have it.

> +		pps_sdp->pps_payload.rc_buf_thresh[i] =
> +			dsc_cfg->rc_buf_thresh[i];
> +
> +	/* PPS 58 - 87 */
> +	/*
> +	 * For DSC sink programming the RC Range parameter fields
> +	 * are as follows: Min_qp[15:11], max_qp[10:6], offset[5:0]
> +	 */
> +	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> +		pps_sdp->pps_payload.rc_range_parameters[i] =
> +			(u16)((dsc_cfg->rc_range_params[i].range_min_qp <<
> +			       DSC_PPS_RC_RANGE_MINQP_SHIFT) |
> +			      (dsc_cfg->rc_range_params[i].range_max_qp <<
> +			       DSC_PPS_RC_RANGE_MAXQP_SHIFT) |
> +			      (dsc_cfg->rc_range_params[i].range_bpg_offset));
> +		pps_sdp->pps_payload.rc_range_parameters[i] =
> +			cpu_to_be16(pps_sdp->pps_payload.rc_range_parameters[i]);
> +	}
> +
> +	/* PPS 88 */
> +	pps_sdp->pps_payload.native_422_420 = (u8)dsc_cfg->native_422 |
> +		(u8)dsc_cfg->native_420 << DSC_PPS_NATIVE_420_SHIFT;
> +
> +	/* PPS 89 */
> +	pps_sdp->pps_payload.second_line_bpg_offset =
> +		(u8)dsc_cfg->second_line_bpg_offset;
> +
> +	/* PPS 90, 91 */
> +	pps_sdp->pps_payload.nsl_bpg_offset =
> +		cpu_to_be16(dsc_cfg->nsl_bpg_offset);
> +
> +	/* PPS 92, 93 */
> +	pps_sdp->pps_payload.second_line_offset_adj =
> +		cpu_to_be16(dsc_cfg->second_line_offset_adj);
> +
> +	/* PPS 94 - 127 are O */
> +}
> +EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index 986f962f38d4..0e5e3368d645 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -8,6 +8,7 @@
>  #ifndef DRM_DSC_H_
>  #define DRM_DSC_H_
>  
> +#include <linux/byteorder/generic.h>

This header doesn't need that AFAICS.

>  #include <drm/drm_dp_helper.h>
>  
>  /* VESA Display Stream Compression DSC 1.2 constants */
> @@ -24,6 +25,23 @@
>  #define DSC_RC_TGT_OFFSET_HI_CONST	    3
>  #define DSC_RC_TGT_OFFSET_LO_CONST	    3
>  
> +/* DSC PPS constants and macros */
> +#define DSC_PPS_VERSION_MAJOR_SHIFT		4
> +#define DSC_PPS_BPC_SHIFT			4
> +#define DSC_PPS_MSB_SHIFT			8
> +#define DSC_PPS_LSB_MASK			(0xFF << 0)
> +#define DSC_PPS_BPP_HIGH_MASK			(0x3 << 8)
> +#define DSC_PPS_VBR_EN_SHIFT			2
> +#define DSC_PPS_SIMPLE422_SHIFT			3
> +#define DSC_PPS_CONVERT_RGB_SHIFT		4
> +#define DSC_PPS_BLOCK_PRED_EN_SHIFT		5
> +#define DSC_PPS_INIT_XMIT_DELAY_HIGH_MASK	(0x3 << 8)
> +#define DSC_PPS_SCALE_DEC_INT_HIGH_MASK		(0xF << 8)
> +#define DSC_PPS_RC_TGT_OFFSET_HI_SHIFT		4
> +#define DSC_PPS_RC_RANGE_MINQP_SHIFT		11
> +#define DSC_PPS_RC_RANGE_MAXQP_SHIFT		6
> +#define DSC_PPS_NATIVE_420_SHIFT		1
> +
>  /* Configuration for a single Rate Control model range */
>  struct dsc_rc_range_parameters {
>  	/* Min Quantization Parameters allowed for this range */
> @@ -463,4 +481,8 @@ struct drm_dsc_pps_infoframe {
>  	struct picture_parameter_set pps_payload;
>  } __packed;
>  
> +void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp);
> +void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> +				struct drm_dsc_config *dsc_cfg);
> +
>  #endif /* _DRM_DSC_H_ */
> -- 
> 2.18.0

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux