Re: [PATCH v8 04/19] drm/dsc: Add helpers for DSC picture parameter set infoframes

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

 




>-----Original Message-----
>From: Navare, Manasi D
>Sent: Friday, November 2, 2018 2:31 PM
>To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
>Cc: Navare, Manasi D <manasi.d.navare@xxxxxxxxx>; Jani Nikula
><jani.nikula@xxxxxxxxxxxxxxx>; Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>;
>Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; Harry Wentland
><harry.wentland@xxxxxxx>
>Subject: [PATCH v8 04/19] drm/dsc: Add helpers for DSC picture parameter set
>infoframes
>
>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.
>
>v7:
>* Use BUILD_BUG_ON() to protect changing struct size (Ville)
>* Remove typecaseting (Ville)
>* Include byteorder.h in drm_dsc.c (Ville)
>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             | 228 ++++++++++++++++++++++++++
> include/drm/drm_dsc.h                 |  21 +++
> 4 files changed, 262 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..3a4942c1ae3b
>--- /dev/null
>+++ b/drivers/gpu/drm/drm_dsc.c
>@@ -0,0 +1,228 @@
>+// SPDX-License-Identifier: MIT

Nit-
In some places the License is within /* */ like comment....
Is the right way to Add License Identifier?

>+/*
>+ * 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 <linux/byteorder/generic.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,
>+				const struct drm_dsc_config *dsc_cfg) {
>+	int i;
>+
>+	/*Protect against someone accidently changing struct size */
           Nit -   ^^^ Missing space after /*

Other than these nits, the patch looks good.

Reviewed-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx?

>+	BUILD_BUG_ON(sizeof(pps_sdp->pps_payload) !=
>+		     DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1 + 1);
>+
>+	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 =
>+		((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
>+		 DSC_PPS_MSB_SHIFT) |
>+		dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
>+		dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
>+		dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
>+		dsc_cfg->block_pred_enable <<
>DSC_PPS_BLOCK_PRED_EN_SHIFT;
>+
>+	/* PPS 5 */
>+	pps_sdp->pps_payload.bits_per_pixel_low =
>+		(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 =
>+		((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 =
>+		(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 =
>+		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 =
>+		((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 =
>+		(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 =
>+		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 = dsc_cfg->flatness_min_qp;
>+
>+	/* PPS 37 */
>+	pps_sdp->pps_payload.flatness_max_qp = dsc_cfg->flatness_max_qp;
>+
>+	/* PPS 38, 39 */
>+	pps_sdp->pps_payload.rc_model_size =
>+		cpu_to_be16(DSC_RC_MODEL_SIZE_CONST);
>+
>+	/* PPS 40 */
>+	pps_sdp->pps_payload.rc_edge_factor =
>DSC_RC_EDGE_FACTOR_CONST;
>+
>+	/* PPS 41 */
>+	pps_sdp->pps_payload.rc_quant_incr_limit0 =
>+		dsc_cfg->rc_quant_incr_limit0;
>+
>+	/* PPS 42 */
>+	pps_sdp->pps_payload.rc_quant_incr_limit1 =
>+		dsc_cfg->rc_quant_incr_limit1;
>+
>+	/* PPS 43 */
>+	pps_sdp->pps_payload.rc_tgt_offset = DSC_RC_TGT_OFFSET_LO_CONST
>|
>+		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++)
>+		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] =
>+			((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 = dsc_cfg->native_422 |
>+		dsc_cfg->native_420 << DSC_PPS_NATIVE_420_SHIFT;
>+
>+	/* PPS 89 */
>+	pps_sdp->pps_payload.second_line_bpg_offset =
>+		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
>b88e31bd9da7..52e57ceaff80 100644
>--- a/include/drm/drm_dsc.h
>+++ b/include/drm/drm_dsc.h
>@@ -24,6 +24,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
>drm_dsc_rc_range_parameters {
> 	/* Min Quantization Parameters allowed for this range */ @@ -458,4
>+475,8 @@ struct drm_dsc_pps_infoframe {
> 	struct drm_dsc_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,
>+				const struct drm_dsc_config *dsc_cfg);
>+
> #endif /* _DRM_DSC_H_ */
>--
>2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux