Re: [PATCH v2] drm: Add DPCD definitions for DP 1.4 DSC feature

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

 



On Wed, Feb 22, 2017 at 10:23:18AM +0200, Jani Nikula wrote:
> 
> [Your MUA messed up the quoting, FTFY below.]
> 
> On Wed, 22 Feb 2017, "Navare, Manasi D" <manasi.d.navare@xxxxxxxxx> wrote:
> > > On Fri, 17 Feb 2017, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote:
> > >> Display stream compression is supported on DP 1.4 DP devices. This 
> > >> patch adds the corersponding DPCD register definitions for DSC.
> > >>
> > >> v2:
> > >> * Rebased on drm-tip
> > >>
> > >> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > >> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > >> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > >> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > >
> > > Tedious work to cross check this stuff against the spec... found one
> > > real issue; while at it please fix a few nitpicks that I would
> > > otherwise have ignored.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > >> ---
> > >>  include/drm/drm_dp_helper.h | 102 
> > >> ++++++++++++++++++++++++++++++++++++++++++++
> > >>  1 file changed, 102 insertions(+)
> > >>
> > >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h 
> > >> index ba89295..4c1fc26 100644
> > >> --- a/include/drm/drm_dp_helper.h
> > >> +++ b/include/drm/drm_dp_helper.h
> > >> @@ -179,6 +179,108 @@
> > >>  
> > >>  #define DP_GUID				    0x030   /* 1.2 */
> > >>  
> > >> +#define DP_DSC_SUPPORT                      0x060   /* DP 1.4 */
> > >> +# define DP_DSC_DECOMPRESSION_IS_SUPPORTED  (1 << 0)
> > >> +
> > >> +#define DP_DSC_REV                          0x061
> > >> +# define DP_DSC_MAJOR_MASK                  (15 << 0)
> > >> +# define DP_DSC_MINOR_MASK                  (15 << 4)
> > >> +# define DP_DSC_MINOR_SHIFT                 4
> > >
> > > Nitpick: Hex is preferred for masks. Same for all masks
> > > below. MAJOR_SHIFT for completeness.
> > >
> > So should I add the MAJOR_SHIFT as well even though it will just 0?
> 
> Yes, 0 is not special here. Having it is also self-documenting, so you
> don't have to look up the DP spec to check.
> 
> > >
> > >
> > >> +
> > >> +#define DP_DSC_RC_BUF_BLK_SIZE              0x062
> > >> +# define DP_DSC_RC_BUF_BLK_SIZE_1           0x0
> > >> +# define DP_DSC_RC_BUF_BLK_SIZE_4           0x1
> > >> +# define DP_DSC_RC_BUF_BLK_SIZE_16          0x2
> > >> +# define DP_DSC_RC_BUF_BLK_SIZE_64          0x3
> > >> +
> > >> +#define DP_DSC_RC_BUF_SIZE                  0x063
> > >> +
> > >> +#define DP_DSC_SLICE_CAP_1                  0x064
> > >> +# define DP_DSC_1_PER_DP_DSC_SINK           (1 << 0)
> > >> +# define DP_DSC_2_PER_DP_DSC_SINK           (1 << 1)
> > >> +# define DP_DSC_4_PER_DP_DSC_SINK           (1 << 3)
> > >> +# define DP_DSC_6_PER_DP_DSC_SINK           (1 << 4)
> > >> +# define DP_DSC_8_PER_DP_DSC_SINK           (1 << 5)
> > >> +# define DP_DSC_10_PER_DP_DSC_SINK          (1 << 6)
> > >> +# define DP_DSC_12_PER_DP_DSC_SINK          (1 << 7)
> > >> +
> > >> +#define DP_DSC_LINE_BUF_BIT_DEPTH           0x065
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_MASK     (15 << 0)
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_9        0x0
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_10       0x1
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_11       0x2
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_12       0x3
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_13       0x4
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_14       0x5
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_15       0x6
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_16       0x7
> > >> +# define DP_DSC_LINE_BUF_BIT_DEPTH_8        0x8
> > >> +
> > >> +#define DP_DSC_BLK_PREDICTION_SUPPORT       0x066
> > >> +# define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0)
> > >> +
> > >> +#define DP_DSC_MAX_BITS_PER_PIXEL_LOW       0x067   /* eDP 1.4 */
> > >> +
> > >> +#define DP_DSC_MAX_BITS_PER_PIXEL_HI        0x068   /* eDP 1.4 */
> > >> +
> > >> +#define DP_DSC_DEC_COLOR_FORMAT_CAP         0x069
> > >> +# define DP_DSC_RGB                         (1 << 0)
> > >> +# define DP_DSC_YCbCr444                    (1 << 1)
> > >> +# define DP_DSC_YCbCr422_Simple             (1 << 2)
> > >> +# define DP_DSC_YCbCr422_Native             (1 << 3)
> > >> +# define DP_DSC_YCbCr420_Native             (1 << 4)
> > >> +
> > >> +#define DP_DSC_DEC_COLOR_DEPTH_CAP          0x06A
> > >> +# define DP_DSC_8_BPC                       (1 << 1)
> > >> +# define DP_DSC_10_BPC                      (1 << 2)
> > >> +# define DP_DSC_12_BPC                      (1 >> 3)
> > >
> > > Oops, shifting to wrong direction!
> >
> > Oops, yes that is my mistake, I will fix the shift direction.
> >
> > >> +
> > >> +#define DP_DSC_PEAK_THROUGHPUT              0x06B
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_340       0x1
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_400       0x2
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_450       0x3
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_500       0x4
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_550       0x5
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_600       0x6
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_650       0x7
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_700       0x8
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_750       0x9
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_800       0xA
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_850       0xB
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_900       0xC
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_950       0xD
> > >> +# define DP_DSC_THROUGHPUT_MODE_0_1000      0xE
> > >
> > > Nitpick: MODE_0_MASK and MODE_0_SHIFT for completeness. Seems
> > > inconsistent to use hex for MODE_0 values and dec for MODE_1 values.
> >
> > For Mode 0, it would be all these values shifted by 0, should I add
> > the shifts by 0 for consistency with MODE_1?
> 
> Nah, that I think is not needed, but I could go both ways. (However for
> defining bits we usually use BIT(0) or (1 << 0) rather than plain 1.)
>

So in case of throughput mode 0, its not bit defs but values
Eg: mode 400 is 2 and mode 450 is 3, so in this case should it be (1 << 0) and (2 << 0)
and (3 << 0) so that its consistent with the Mode1 decimals with shift of 0?

Regards
Manasi

 
> > And yes I will add MODE_0 MASK and SHIFT.
> 
> Thanks.
> 
> BR,
> Jani.
> 
> >
> > Regards
> > Manasi
> > >
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_MASK      (15 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_SHIFT     4
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_340       (1 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_400       (2 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_450       (3 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_500       (4 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_550       (5 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_600       (6 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_650       (7 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_700       (8 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_750       (9 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_800       (10 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_850       (11 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_900       (12 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_950       (13 << 4)
> > >> +# define DP_DSC_THROUGHPUT_MODE_1_1000      (14 << 4)
> > >> +
> > >> +#define DP_DSC_MAX_SLICE_WIDTH              0x06C
> > >> +
> > >> +#define DP_DSC_SLICE_CAP_2                  0x06D
> > >> +# define DP_DSC_16_PER_DP_DSC_SINK          (1 << 0)
> > >> +# define DP_DSC_20_PER_DP_DSC_SINK          (1 << 1)
> > >> +# define DP_DSC_24_PER_DP_DSC_SINK          (1 << 2)
> > >> +
> > >> +#define DP_DSC_BITS_PER_PIXEL_INC           0x06F
> > >> +# define DP_DSC_BITS_PER_PIXEL_1_16         0x0
> > >> +# define DP_DSC_BITS_PER_PIXEL_1_8          0x1
> > >> +# define DP_DSC_BITS_PER_PIXEL_1_4          0x2
> > >> +# define DP_DSC_BITS_PER_PIXEL_1_2          0x3
> > >> +# define DP_DSC_BITS_PER_PIXEL_1            0x4
> > >> +
> > >>  #define DP_PSR_SUPPORT                      0x070   /* XXX 1.2? */
> > >>  # define DP_PSR_IS_SUPPORTED                1
> > >>  # define DP_PSR2_IS_SUPPORTED		    2	    /* eDP 1.4 */
> > >
> > > --
> > > Jani Nikula, Intel Open Source Technology Center
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux