Re: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS infoframe
Thanks for the review.
And yes it just removes the bitfields in PPS since the packing of the
bitfields and memory allocations are heavily dependent on HW.
So we cant have them in DRM level.
I will make sure to add this in version history and also add
Suggested-by: jani.nik...@linux.intel.com
Manasi
On Thu, Aug 23, 2018 at 03:40:12PM -0400, Harry Wentland wrote:
> On 2018-07-31 05:07 PM, Manasi Navare wrote:
> > This patch defines a new header file for all the DSC 1.2 structures
> > and creates a structure for PPS infoframe which will be used to send
> > picture parameter set secondary data packet for display stream compression.
> > All the PPS infoframe syntax elements are taken from DSC 1.2 specification
> > from VESA.
> >
> > Cc: Gaurav K Singh
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Jani Nikula
> > Cc: Ville Syrjala
> > Cc: Anusha Srivatsa
> > Cc: Harry Wentland
> > Signed-off-by: Manasi Navare
>
> Looks like this version basically removes the bitfield definitions and adds
> those
> in the comments, compared to the review in May.
>
> Reviewed-by: Harry Wentland
>
> Harry
>
> > ---
> > include/drm/drm_dsc.h | 365
> > ++
> > 1 file changed, 365 insertions(+)
> > create mode 100644 include/drm/drm_dsc.h
> >
> > diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> > new file mode 100644
> > index 000..678e8e6
> > --- /dev/null
> > +++ b/include/drm/drm_dsc.h
> > @@ -0,0 +1,365 @@
> > +/*
> > + * Copyright (C) 2018 Intel Corp.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + *
> > + * Authors:
> > + * Manasi Navare
> > + */
> > +
> > +#ifndef DRM_DSC_H_
> > +#define DRM_DSC_H_
> > +
> > +#include
> > +
> > +/* VESA Display Stream Compression DSC 1.2 constants */
> > +#define DSC_NUM_BUF_RANGES 15
> > +
> > +/**
> > + * struct picture_parameter_set - Represents 128 bytes of Picture
> > Parameter Set
> > + *
> > + * The VESA DSC standard defines picture parameter set (PPS) which display
> > + * stream compression encoders must communicate to decoders.
> > + * The PPS is encapsulated in 128 bytes (PPS 0 through PPS 127). The
> > fields in
> > + * this structure are as per Table 4.1 in Vesa DSC specification v1.1/v1.2.
> > + * The PPS fields that span over more than a byte should be stored in Big
> > Endian
> > + * format.
> > + */
> > +struct picture_parameter_set {
> > + /**
> > +* @dsc_version:
> > +* PPS0[3:0] - dsc_version_minor: Contains Minor version of DSC
> > +* PPS0[7:4] - dsc_version_major: Contains major version of DSC
> > +*/
> > + u8 dsc_version;
> > + /**
> > +* @pps_identifier:
> > +* PPS1[7:0] - Application specific identifier that can be
> > +* used to differentiate between different PPS tables.
> > +*/
> > + u8 pps_identifier;
> > + /**
> > +* @pps_reserved:
> > +* PPS2[7:0]- RESERVED Byte
> > +*/
> > + u8 pps_reserved;
> > + /**
> > +* @pps_3:
> > +* PPS3[3:0] - linebuf_depth: Contains linebuffer bit depth used to
> > +* generate the bitstream. (0x0 - 16 bits for DSC 1.2, 0x8 - 8 bits,
> > +* 0xA - 10 bits, 0xB - 11 bits, 0xC - 12 bits, 0xD - 13 bits,
> > +* 0xE - 14 bits for DSC1.2, 0xF - 14 bits for DSC 1.2.
> > +* PPS3[7:4] - bits_per_component: Bits per component for the original
> > +* pixels of the encoded picture.
> > +* 0x0 = 16bpc (allowed only when dsc_version_minor = 0x2)
> > +* 0x8 = 8bpc, 0xA = 10bpc, 0xC = 12bpc, 0xE = 14bpc (also
> > +* allowed only when dsc_minor_version = 0x2)
> > +*/
> > + u8 pps_3;
> > + /**
> > +* @pps_4:
> > +* PPS4[1:0] -These are the most significant 2 bits of
> > +* compressed BPP bits_per_pixel[9:0] syntax element.
> > +* PPS4[2] - vbr_enable: 0 = VBR disabled, 1 = VBR enabled
> > +* PPS4[3] - simple_422:
Re: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS infoframe
On Fri, Aug 17, 2018 at 12:31:51PM -0700, Srivatsa, Anusha wrote:
> This patch needs to now incorporate the newly added slice_row_per_frame
> parameter in PPS_16.
Nope, the slice_row_per_frame and slice_per_line are only required to configure
PPS
on the source side. They are not DSC spec related PPS parameters.
Manasi
>
> Anusha
>
> >-Original Message-
> >From: Navare, Manasi D
> >Sent: Tuesday, July 31, 2018 2:07 PM
> >To: intel-...@lists.freedesktop.org
> >Cc: Navare, Manasi D ; Singh, Gaurav K
> >; dri-devel@lists.freedesktop.org; Jani Nikula
> >; Ville Syrjala ;
> >Srivatsa, Anusha ; Harry Wentland
> >
> >Subject: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS
> >infoframe
> >
> >This patch defines a new header file for all the DSC 1.2 structures and
> >creates a
> >structure for PPS infoframe which will be used to send picture parameter set
> >secondary data packet for display stream compression.
> >All the PPS infoframe syntax elements are taken from DSC 1.2 specification
> >from
> >VESA.
> >
> >Cc: Gaurav K Singh
> >Cc: dri-devel@lists.freedesktop.org
> >Cc: Jani Nikula
> >Cc: Ville Syrjala
> >Cc: Anusha Srivatsa
> >Cc: Harry Wentland
> >Signed-off-by: Manasi Navare
> >---
> > include/drm/drm_dsc.h | 365
> >++
> > 1 file changed, 365 insertions(+)
> > create mode 100644 include/drm/drm_dsc.h
> >
> >diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h new file mode
> >100644 index 000..678e8e6
> >--- /dev/null
> >+++ b/include/drm/drm_dsc.h
> >@@ -0,0 +1,365 @@
> >+/*
> >+ * Copyright (C) 2018 Intel Corp.
> >+ *
> >+ * Permission is hereby granted, free of charge, to any person
> >+obtaining a
> >+ * copy of this software and associated documentation files (the
> >+"Software"),
> >+ * to deal in the Software without restriction, including without
> >+limitation
> >+ * the rights to use, copy, modify, merge, publish, distribute,
> >+sublicense,
> >+ * and/or sell copies of the Software, and to permit persons to whom
> >+the
> >+ * Software is furnished to do so, subject to the following conditions:
> >+ *
> >+ * The above copyright notice and this permission notice shall be
> >+included in
> >+ * all copies or substantial portions of the Software.
> >+ *
> >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >+EXPRESS OR
> >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >+MERCHANTABILITY,
> >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> >EVENT
> >+SHALL
> >+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> >+DAMAGES OR
> >+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> >+OTHERWISE,
> >+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> >USE
> >+OR
> >+ * OTHER DEALINGS IN THE SOFTWARE.
> >+ *
> >+ * Authors:
> >+ * Manasi Navare */
> >+
> >+#ifndef DRM_DSC_H_
> >+#define DRM_DSC_H_
> >+
> >+#include
> >+
> >+/* VESA Display Stream Compression DSC 1.2 constants */
> >+#define DSC_NUM_BUF_RANGES 15
> >+
> >+/**
> >+ * struct picture_parameter_set - Represents 128 bytes of Picture
> >+Parameter Set
> >+ *
> >+ * The VESA DSC standard defines picture parameter set (PPS) which
> >+display
> >+ * stream compression encoders must communicate to decoders.
> >+ * The PPS is encapsulated in 128 bytes (PPS 0 through PPS 127). The
> >+fields in
> >+ * this structure are as per Table 4.1 in Vesa DSC specification v1.1/v1.2.
> >+ * The PPS fields that span over more than a byte should be stored in
> >+Big Endian
> >+ * format.
> >+ */
> >+struct picture_parameter_set {
> >+/**
> >+ * @dsc_version:
> >+ * PPS0[3:0] - dsc_version_minor: Contains Minor version of DSC
> >+ * PPS0[7:4] - dsc_version_major: Contains major version of DSC
> >+ */
> >+u8 dsc_version;
> >+/**
> >+ * @pps_identifier:
> >+ * PPS1[7:0] - Application specific identifier that can be
> >+ * used to differentiate between different PPS tables.
> >+ */
> >+u8 pps_identifier;
> >+/**
> >+ * @pps_reserved:
> >+ * PPS2[7:0]- RESERVED Byte
> >+ */
> >+u8 pps_reserved;
> >+/**
> >+ * @pps_3:
> >+ * PPS3[3:0] - linebuf_depth: Contains linebuffer bit depth used to
> >+ * generate the bitstream. (0x0 - 16 bits for DSC 1.2, 0x8 - 8 bits,
> >+ * 0xA - 10 bits, 0xB - 11 bits, 0xC - 12 bits, 0xD - 13 bits,
> >+ * 0xE - 14 bits for DSC1.2, 0xF - 14 bits for DSC 1.2.
> >+ * PPS3[7:4] - bits_per_component: Bits per component for the original
> >+ * pixels of the encoded picture.
> >+ * 0x0 = 16bpc (allowed only when dsc_version_minor = 0x2)
> >+ * 0x8 = 8bpc, 0xA = 10bpc, 0xC = 12bpc, 0xE = 14bpc (also
> >+ * allowed only when dsc_minor_version = 0x2)
> >+ */
> >+u8 pps_3;
> >+/**
> >+ * @pps_4:
> >+ * PPS4[1:0] -These are the most significant 2 bits of
> >+ * compressed BPP bits_per_pixel[9:0] syntax element.
> >+ *
Re: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS infoframe
On 2018-07-31 05:07 PM, Manasi Navare wrote:
> This patch defines a new header file for all the DSC 1.2 structures
> and creates a structure for PPS infoframe which will be used to send
> picture parameter set secondary data packet for display stream compression.
> All the PPS infoframe syntax elements are taken from DSC 1.2 specification
> from VESA.
>
> Cc: Gaurav K Singh
> Cc: dri-devel@lists.freedesktop.org
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Anusha Srivatsa
> Cc: Harry Wentland
> Signed-off-by: Manasi Navare
Looks like this version basically removes the bitfield definitions and adds
those
in the comments, compared to the review in May.
Reviewed-by: Harry Wentland
Harry
> ---
> include/drm/drm_dsc.h | 365
> ++
> 1 file changed, 365 insertions(+)
> create mode 100644 include/drm/drm_dsc.h
>
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> new file mode 100644
> index 000..678e8e6
> --- /dev/null
> +++ b/include/drm/drm_dsc.h
> @@ -0,0 +1,365 @@
> +/*
> + * Copyright (C) 2018 Intel Corp.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Manasi Navare
> + */
> +
> +#ifndef DRM_DSC_H_
> +#define DRM_DSC_H_
> +
> +#include
> +
> +/* VESA Display Stream Compression DSC 1.2 constants */
> +#define DSC_NUM_BUF_RANGES 15
> +
> +/**
> + * struct picture_parameter_set - Represents 128 bytes of Picture Parameter
> Set
> + *
> + * The VESA DSC standard defines picture parameter set (PPS) which display
> + * stream compression encoders must communicate to decoders.
> + * The PPS is encapsulated in 128 bytes (PPS 0 through PPS 127). The fields
> in
> + * this structure are as per Table 4.1 in Vesa DSC specification v1.1/v1.2.
> + * The PPS fields that span over more than a byte should be stored in Big
> Endian
> + * format.
> + */
> +struct picture_parameter_set {
> + /**
> + * @dsc_version:
> + * PPS0[3:0] - dsc_version_minor: Contains Minor version of DSC
> + * PPS0[7:4] - dsc_version_major: Contains major version of DSC
> + */
> + u8 dsc_version;
> + /**
> + * @pps_identifier:
> + * PPS1[7:0] - Application specific identifier that can be
> + * used to differentiate between different PPS tables.
> + */
> + u8 pps_identifier;
> + /**
> + * @pps_reserved:
> + * PPS2[7:0]- RESERVED Byte
> + */
> + u8 pps_reserved;
> + /**
> + * @pps_3:
> + * PPS3[3:0] - linebuf_depth: Contains linebuffer bit depth used to
> + * generate the bitstream. (0x0 - 16 bits for DSC 1.2, 0x8 - 8 bits,
> + * 0xA - 10 bits, 0xB - 11 bits, 0xC - 12 bits, 0xD - 13 bits,
> + * 0xE - 14 bits for DSC1.2, 0xF - 14 bits for DSC 1.2.
> + * PPS3[7:4] - bits_per_component: Bits per component for the original
> + * pixels of the encoded picture.
> + * 0x0 = 16bpc (allowed only when dsc_version_minor = 0x2)
> + * 0x8 = 8bpc, 0xA = 10bpc, 0xC = 12bpc, 0xE = 14bpc (also
> + * allowed only when dsc_minor_version = 0x2)
> + */
> + u8 pps_3;
> + /**
> + * @pps_4:
> + * PPS4[1:0] -These are the most significant 2 bits of
> + * compressed BPP bits_per_pixel[9:0] syntax element.
> + * PPS4[2] - vbr_enable: 0 = VBR disabled, 1 = VBR enabled
> + * PPS4[3] - simple_422: Indicates if decoder drops samples to
> + * reconstruct the 4:2:2 picture.
> + * PPS4[4] - Convert_rgb: Indicates if DSC color space conversion is
> + * active.
> + * PPS4[5] - blobk_pred_enable: Indicates if BP is used to code any
> + * groups in picture
> + * PPS4[7:6] - Reseved bits
> + */
> + u8 pps_4;
> + /**
> + * @bits_per_pixel_low:
> + * PPS5[7:0] - This indicates the lower significant 8 bits of
> + * the compressed BPP bits_per_pixel[9:0] element.
> + */
> + u8
RE: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS infoframe
This patch needs to now incorporate the newly added slice_row_per_frame
parameter in PPS_16.
Anusha
>-Original Message-
>From: Navare, Manasi D
>Sent: Tuesday, July 31, 2018 2:07 PM
>To: intel-...@lists.freedesktop.org
>Cc: Navare, Manasi D ; Singh, Gaurav K
>; dri-devel@lists.freedesktop.org; Jani Nikula
>; Ville Syrjala ;
>Srivatsa, Anusha ; Harry Wentland
>
>Subject: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS
>infoframe
>
>This patch defines a new header file for all the DSC 1.2 structures and
>creates a
>structure for PPS infoframe which will be used to send picture parameter set
>secondary data packet for display stream compression.
>All the PPS infoframe syntax elements are taken from DSC 1.2 specification from
>VESA.
>
>Cc: Gaurav K Singh
>Cc: dri-devel@lists.freedesktop.org
>Cc: Jani Nikula
>Cc: Ville Syrjala
>Cc: Anusha Srivatsa
>Cc: Harry Wentland
>Signed-off-by: Manasi Navare
>---
> include/drm/drm_dsc.h | 365
>++
> 1 file changed, 365 insertions(+)
> create mode 100644 include/drm/drm_dsc.h
>
>diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h new file mode
>100644 index 000..678e8e6
>--- /dev/null
>+++ b/include/drm/drm_dsc.h
>@@ -0,0 +1,365 @@
>+/*
>+ * Copyright (C) 2018 Intel Corp.
>+ *
>+ * Permission is hereby granted, free of charge, to any person
>+obtaining a
>+ * copy of this software and associated documentation files (the
>+"Software"),
>+ * to deal in the Software without restriction, including without
>+limitation
>+ * the rights to use, copy, modify, merge, publish, distribute,
>+sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom
>+the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be
>+included in
>+ * all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>+EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>+MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
>EVENT
>+SHALL
>+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>+DAMAGES OR
>+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>+OTHERWISE,
>+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>USE
>+OR
>+ * OTHER DEALINGS IN THE SOFTWARE.
>+ *
>+ * Authors:
>+ * Manasi Navare */
>+
>+#ifndef DRM_DSC_H_
>+#define DRM_DSC_H_
>+
>+#include
>+
>+/* VESA Display Stream Compression DSC 1.2 constants */
>+#define DSC_NUM_BUF_RANGES15
>+
>+/**
>+ * struct picture_parameter_set - Represents 128 bytes of Picture
>+Parameter Set
>+ *
>+ * The VESA DSC standard defines picture parameter set (PPS) which
>+display
>+ * stream compression encoders must communicate to decoders.
>+ * The PPS is encapsulated in 128 bytes (PPS 0 through PPS 127). The
>+fields in
>+ * this structure are as per Table 4.1 in Vesa DSC specification v1.1/v1.2.
>+ * The PPS fields that span over more than a byte should be stored in
>+Big Endian
>+ * format.
>+ */
>+struct picture_parameter_set {
>+ /**
>+ * @dsc_version:
>+ * PPS0[3:0] - dsc_version_minor: Contains Minor version of DSC
>+ * PPS0[7:4] - dsc_version_major: Contains major version of DSC
>+ */
>+ u8 dsc_version;
>+ /**
>+ * @pps_identifier:
>+ * PPS1[7:0] - Application specific identifier that can be
>+ * used to differentiate between different PPS tables.
>+ */
>+ u8 pps_identifier;
>+ /**
>+ * @pps_reserved:
>+ * PPS2[7:0]- RESERVED Byte
>+ */
>+ u8 pps_reserved;
>+ /**
>+ * @pps_3:
>+ * PPS3[3:0] - linebuf_depth: Contains linebuffer bit depth used to
>+ * generate the bitstream. (0x0 - 16 bits for DSC 1.2, 0x8 - 8 bits,
>+ * 0xA - 10 bits, 0xB - 11 bits, 0xC - 12 bits, 0xD - 13 bits,
>+ * 0xE - 14 bits for DSC1.2, 0xF - 14 bits for DSC 1.2.
>+ * PPS3[7:4] - bits_per_component: Bits per component for the original
>+ * pixels of the encoded picture.
>+ * 0x0 = 16bpc (allowed only when dsc_version_minor = 0x2)
>+ * 0x8 = 8bpc, 0xA = 10bpc, 0xC = 12bpc, 0xE = 14bpc (also
>+ * allowed only when dsc_minor_version = 0x2)
>+ */
>+ u8 pps_3;
>+ /**
>+ * @pps_4:
>+ * PPS4[1:0] -These are the most significant 2 bits of
>+ * compressed BPP bits_per_pixel[9:0] syntax element.
>+ * PPS4[2] - vbr_enable: 0 = VBR disabled, 1 = VBR enabled
>+ * PPS4[3] - simple_422: Indicates if decoder drops samples to
>+ * reconstruct the 4:2:2 picture.
>+ * PPS4[4] - Convert_rgb: Indicates if DSC color space conversion is
>+ * active.
>+ * PPS4[5] - blobk_pred_enable: Indicates if BP is used to code any
>+ * groups in picture
>+ * PPS4[7:6] - Reseved bits
>+ */
>+ u8 pps_4;
>+
