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

2018-11-06 Thread Manasi Navare 
On Mon, Nov 05, 2018 at 05:38:19PM -0800, Srivatsa, Anusha wrote:
> 
> 
> >-Original Message-
> >From: Navare, Manasi D
> >Sent: Friday, November 2, 2018 2:31 PM
> >To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> >Cc: Navare, Manasi D ; Jani Nikula
> >; Ville Syrjala ;
> >Srivatsa, Anusha ; Harry Wentland
> >
> >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@lists.freedesktop.org
> >Cc: Jani Nikula 
> >Cc: Ville Syrjala 
> >Cc: Anusha Srivatsa 
> >Cc: Harry Wentland 
> >Signed-off-by: Manasi Navare 
> >Acked-by: Harry Wentland 
> >---
> > 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 ..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?

Yes /*  */ was giving me checkpatch warning.

> 
> >+/*
> >+ * Copyright © 2018 Intel Corp
> >+ *
> >+ * Author:
> >+ * Manasi Navare   */
> >+
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+
> >+/**
> >+ * DOC: dsc helpers
> >+ *
> >+ * These functions contain some common logic and helpers to deal with
> >+VESA
> >+ * Display Stream Compression standard required for DSC on Display
> >+

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

2018-11-05 Thread Srivatsa, Anusha 


>-Original Message-
>From: Navare, Manasi D
>Sent: Friday, November 2, 2018 2:31 PM
>To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>Cc: Navare, Manasi D ; Jani Nikula
>; Ville Syrjala ;
>Srivatsa, Anusha ; Harry Wentland
>
>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@lists.freedesktop.org
>Cc: Jani Nikula 
>Cc: Ville Syrjala 
>Cc: Anusha Srivatsa 
>Cc: Harry Wentland 
>Signed-off-by: Manasi Navare 
>Acked-by: Harry Wentland 
>---
> 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 ..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   */
>+
>+#include 
>+#include 
>+#include 
>+#include 
>+#include 
>+#include 
>+#include 
>+
>+/**
>+ * 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(_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 conf

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

2018-11-02 Thread Manasi Navare 
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@lists.freedesktop.org
Cc: Jani Nikula 
Cc: Ville Syrjala 
Cc: Anusha Srivatsa 
Cc: Harry Wentland 
Signed-off-by: Manasi Navare 
Acked-by: Harry Wentland 
---
 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 ..3a4942c1ae3b
--- /dev/null
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corp
+ *
+ * Author:
+ * Manasi Navare 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * 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(_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 */
+   BUILD_BUG_ON(sizeof(pps_sdp->pps_payload) !=
+DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1 + 1);
+
+   memset(_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 */
+