diff mbox series

[v9,01/24] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities

Message ID 20181114015232.21952-2-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series Remaining DSC + FEC patches | expand

Commit Message

Navare, Manasi Nov. 14, 2018, 1:52 a.m. UTC 
DSC DPCD color depth register advertises its color depth capabilities
by setting each of the bits that corresponding to a specific color
depth. This patch defines those specific color depths and adds
a helper to return an array of color depth capabilities.

Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++----------
 include/drm/drm_dp_helper.h     |  9 +++++----
 2 files changed, 24 insertions(+), 14 deletions(-)

Comments

Ville Syrjala Nov. 19, 2018, 7:43 p.m. UTC | #1 
On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> DSC DPCD color depth register advertises its color depth capabilities
> by setting each of the bits that corresponding to a specific color
> depth. This patch defines those specific color depths and adds
> a helper to return an array of color depth capabilities.
> 
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++----------
>  include/drm/drm_dp_helper.h     |  9 +++++----
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 6d483487f2b4..286567063960 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> +				     u8 *dsc_sink_color_depth_cap)
>  {
> +	int i, cnt = 0;
>  	u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
>  
> -	if (color_depth & DP_DSC_12_BPC)
> -		return 12;
> -	if (color_depth & DP_DSC_10_BPC)
> -		return 10;
> -	if (color_depth & DP_DSC_8_BPC)
> -		return 8;
> -
> -	return 0;
> +	for (i = 1; i <= 3; i++) {
> +		if (!(color_depth & BIT(i)))
> +			continue;
> +		switch (i) {
> +		case 1:
> +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> +			break;
> +		case 2:
> +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> +			break;
> +		case 3:
> +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> +			break;
> +		}
> +	}
>  }
> -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 3314e91f6eb3..ea3233b0a790 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -242,9 +242,9 @@
>  # 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)
> +# define DP_DSC_8_BPC                       8
> +# define DP_DSC_10_BPC                      10
> +# define DP_DSC_12_BPC                      12


I'd suggest something simpler like:

int foo(u8 bpc[3])
{
	int num_bpc = 0;

	if (color_depth & DP_DSC_12_BPC)
		bpc[num_bpc++] = 12;
	if (color_depth & DP_DSC_10_BPC)
		bpc[num_bpc++] = 10;
	if (color_depth & DP_DSC_8_BPC)
		bpc[num_bpc++] = 8;

	return num_bpc;
}

>  
>  #define DP_DSC_PEAK_THROUGHPUT              0x06B
>  # define DP_DSC_THROUGHPUT_MODE_0_MASK      (0xf << 0)
> @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  				   bool is_edp);
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> +				     u8 *dsc_sink_color_depth_cap);
>  
>  static inline bool
>  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> -- 
> 2.19.1
Navare, Manasi Nov. 19, 2018, 8:10 p.m. UTC | #2 
On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > DSC DPCD color depth register advertises its color depth capabilities
> > by setting each of the bits that corresponding to a specific color
> > depth. This patch defines those specific color depths and adds
> > a helper to return an array of color depth capabilities.
> > 
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++----------
> >  include/drm/drm_dp_helper.h     |  9 +++++----
> >  2 files changed, 24 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 6d483487f2b4..286567063960 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> >  
> > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > +				     u8 *dsc_sink_color_depth_cap)
> >  {
> > +	int i, cnt = 0;
> >  	u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
> >  
> > -	if (color_depth & DP_DSC_12_BPC)
> > -		return 12;
> > -	if (color_depth & DP_DSC_10_BPC)
> > -		return 10;
> > -	if (color_depth & DP_DSC_8_BPC)
> > -		return 8;
> > -
> > -	return 0;
> > +	for (i = 1; i <= 3; i++) {
> > +		if (!(color_depth & BIT(i)))
> > +			continue;
> > +		switch (i) {
> > +		case 1:
> > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > +			break;
> > +		case 2:
> > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > +			break;
> > +		case 3:
> > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > +			break;
> > +		}
> > +	}
> >  }
> > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 3314e91f6eb3..ea3233b0a790 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -242,9 +242,9 @@
> >  # 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)
> > +# define DP_DSC_8_BPC                       8
> > +# define DP_DSC_10_BPC                      10
> > +# define DP_DSC_12_BPC                      12
> 
> 
> I'd suggest something simpler like:
> 
> int foo(u8 bpc[3])

Is passing a full array recommended method vs passing the pointer to the array?

> {
> 	int num_bpc = 0;
> 
> 	if (color_depth & DP_DSC_12_BPC)
> 		bpc[num_bpc++] = 12;
> 	if (color_depth & DP_DSC_10_BPC)
> 		bpc[num_bpc++] = 10;
> 	if (color_depth & DP_DSC_8_BPC)
> 		bpc[num_bpc++] = 8;
> 
> 	return num_bpc;
> }

Yes I could modify like above except start from lowest bpc in bpc[0] going all the way to highest.
Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs for now right?

Manasi

> 
> >  
> >  #define DP_DSC_PEAK_THROUGHPUT              0x06B
> >  # define DP_DSC_THROUGHPUT_MODE_0_MASK      (0xf << 0)
> > @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >  				   bool is_edp);
> >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> > +				     u8 *dsc_sink_color_depth_cap);
> >  
> >  static inline bool
> >  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > -- 
> > 2.19.1
> 
> -- 
> Ville Syrjälä
> Intel
Ville Syrjala Nov. 19, 2018, 8:33 p.m. UTC | #3 
On Mon, Nov 19, 2018 at 12:10:47PM -0800, Manasi Navare wrote:
> On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > > DSC DPCD color depth register advertises its color depth capabilities
> > > by setting each of the bits that corresponding to a specific color
> > > depth. This patch defines those specific color depths and adds
> > > a helper to return an array of color depth capabilities.
> > > 
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++----------
> > >  include/drm/drm_dp_helper.h     |  9 +++++----
> > >  2 files changed, 24 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > index 6d483487f2b4..286567063960 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> > >  
> > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > > +				     u8 *dsc_sink_color_depth_cap)
> > >  {
> > > +	int i, cnt = 0;
> > >  	u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
> > >  
> > > -	if (color_depth & DP_DSC_12_BPC)
> > > -		return 12;
> > > -	if (color_depth & DP_DSC_10_BPC)
> > > -		return 10;
> > > -	if (color_depth & DP_DSC_8_BPC)
> > > -		return 8;
> > > -
> > > -	return 0;
> > > +	for (i = 1; i <= 3; i++) {
> > > +		if (!(color_depth & BIT(i)))
> > > +			continue;
> > > +		switch (i) {
> > > +		case 1:
> > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > > +			break;
> > > +		case 2:
> > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > > +			break;
> > > +		case 3:
> > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > > +			break;
> > > +		}
> > > +	}
> > >  }
> > > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > index 3314e91f6eb3..ea3233b0a790 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -242,9 +242,9 @@
> > >  # 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)
> > > +# define DP_DSC_8_BPC                       8
> > > +# define DP_DSC_10_BPC                      10
> > > +# define DP_DSC_12_BPC                      12
> > 
> > 
> > I'd suggest something simpler like:
> > 
> > int foo(u8 bpc[3])
> 
> Is passing a full array recommended method vs passing the pointer to the array?

It's the same thing in C. The compiler will treat both as a pointer
(eg. sadly you can't use ARRAY_SIZE() on this because it will just use
the size of the pointer rather than the size of the array in the
calculation).

Despite the language shortcomings I like to use the array notation
as a means to document what is the expected size of the passed in
array.

> 
> > {
> > 	int num_bpc = 0;
> > 
> > 	if (color_depth & DP_DSC_12_BPC)
> > 		bpc[num_bpc++] = 12;
> > 	if (color_depth & DP_DSC_10_BPC)
> > 		bpc[num_bpc++] = 10;
> > 	if (color_depth & DP_DSC_8_BPC)
> > 		bpc[num_bpc++] = 8;
> > 
> > 	return num_bpc;
> > }
> 
> Yes I could modify like above except start from lowest bpc in bpc[0] going all the way to highest.

Highest to lowest seems more sensible to me since we want to pick the
max. So when we walk the array we can bail as soon as we find the
highest suitable value.

> Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs for now right?

Yes. Adding more would require changing the function anyway,
and so the callers can be updated at the same time.

> 
> Manasi
> 
> > 
> > >  
> > >  #define DP_DSC_PEAK_THROUGHPUT              0x06B
> > >  # define DP_DSC_THROUGHPUT_MODE_0_MASK      (0xf << 0)
> > > @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > >  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > >  				   bool is_edp);
> > >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> > > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> > > +				     u8 *dsc_sink_color_depth_cap);
> > >  
> > >  static inline bool
> > >  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > -- 
> > > 2.19.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel
Navare, Manasi Nov. 19, 2018, 10:11 p.m. UTC | #4 
On Mon, Nov 19, 2018 at 10:33:37PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 19, 2018 at 12:10:47PM -0800, Manasi Navare wrote:
> > On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > > > DSC DPCD color depth register advertises its color depth capabilities
> > > > by setting each of the bits that corresponding to a specific color
> > > > depth. This patch defines those specific color depths and adds
> > > > a helper to return an array of color depth capabilities.
> > > > 
> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++----------
> > > >  include/drm/drm_dp_helper.h     |  9 +++++----
> > > >  2 files changed, 24 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > > index 6d483487f2b4..286567063960 100644
> > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> > > >  
> > > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > > > +				     u8 *dsc_sink_color_depth_cap)
> > > >  {
> > > > +	int i, cnt = 0;
> > > >  	u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
> > > >  
> > > > -	if (color_depth & DP_DSC_12_BPC)
> > > > -		return 12;
> > > > -	if (color_depth & DP_DSC_10_BPC)
> > > > -		return 10;
> > > > -	if (color_depth & DP_DSC_8_BPC)
> > > > -		return 8;
> > > > -
> > > > -	return 0;
> > > > +	for (i = 1; i <= 3; i++) {
> > > > +		if (!(color_depth & BIT(i)))
> > > > +			continue;
> > > > +		switch (i) {
> > > > +		case 1:
> > > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > > > +			break;
> > > > +		case 2:
> > > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > > > +			break;
> > > > +		case 3:
> > > > +			dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > >  }
> > > > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > > > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > > index 3314e91f6eb3..ea3233b0a790 100644
> > > > --- a/include/drm/drm_dp_helper.h
> > > > +++ b/include/drm/drm_dp_helper.h
> > > > @@ -242,9 +242,9 @@
> > > >  # 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)
> > > > +# define DP_DSC_8_BPC                       8
> > > > +# define DP_DSC_10_BPC                      10
> > > > +# define DP_DSC_12_BPC                      12
> > > 
> > > 
> > > I'd suggest something simpler like:
> > > 
> > > int foo(u8 bpc[3])
> > 
> > Is passing a full array recommended method vs passing the pointer to the array?
> 
> It's the same thing in C. The compiler will treat both as a pointer
> (eg. sadly you can't use ARRAY_SIZE() on this because it will just use
> the size of the pointer rather than the size of the array in the
> calculation).
> 
> Despite the language shortcomings I like to use the array notation
> as a means to document what is the expected size of the passed in
> array.
>

Ok will change it to use array notation
 
> > 
> > > {
> > > 	int num_bpc = 0;
> > > 
> > > 	if (color_depth & DP_DSC_12_BPC)
> > > 		bpc[num_bpc++] = 12;
> > > 	if (color_depth & DP_DSC_10_BPC)
> > > 		bpc[num_bpc++] = 10;
> > > 	if (color_depth & DP_DSC_8_BPC)
> > > 		bpc[num_bpc++] = 8;
> > > 
> > > 	return num_bpc;
> > > }
> > 
> > Yes I could modify like above except start from lowest bpc in bpc[0] going all the way to highest.
> 
> Highest to lowest seems more sensible to me since we want to pick the
> max. So when we walk the array we can bail as soon as we find the
> highest suitable value.

Yes so bail as soon as the value is less than or equal to the max determined from the user requested bpc, right?

Manasi

> 
> > Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs for now right?
> 
> Yes. Adding more would require changing the function anyway,
> and so the callers can be updated at the same time.
> 
> > 
> > Manasi
> > 
> > > 
> > > >  
> > > >  #define DP_DSC_PEAK_THROUGHPUT              0x06B
> > > >  # define DP_DSC_THROUGHPUT_MODE_0_MASK      (0xf << 0)
> > > > @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > > >  u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > > >  				   bool is_edp);
> > > >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> > > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> > > > +void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> > > > +				     u8 *dsc_sink_color_depth_cap);
> > > >  
> > > >  static inline bool
> > > >  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > > -- 
> > > > 2.19.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 6d483487f2b4..286567063960 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1428,17 +1428,26 @@  u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
 }
 EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
 
-u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
+				     u8 *dsc_sink_color_depth_cap)
 {
+	int i, cnt = 0;
 	u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
 
-	if (color_depth & DP_DSC_12_BPC)
-		return 12;
-	if (color_depth & DP_DSC_10_BPC)
-		return 10;
-	if (color_depth & DP_DSC_8_BPC)
-		return 8;
-
-	return 0;
+	for (i = 1; i <= 3; i++) {
+		if (!(color_depth & BIT(i)))
+			continue;
+		switch (i) {
+		case 1:
+			dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
+			break;
+		case 2:
+			dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
+			break;
+		case 3:
+			dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
+			break;
+		}
+	}
 }
-EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
+EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 3314e91f6eb3..ea3233b0a790 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -242,9 +242,9 @@ 
 # 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)
+# define DP_DSC_8_BPC                       8
+# define DP_DSC_10_BPC                      10
+# define DP_DSC_12_BPC                      12
 
 #define DP_DSC_PEAK_THROUGHPUT              0x06B
 # define DP_DSC_THROUGHPUT_MODE_0_MASK      (0xf << 0)
@@ -1123,7 +1123,8 @@  drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
 				   bool is_edp);
 u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
-u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
+void drm_dp_dsc_sink_color_depth_cap(const u8 dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
+				     u8 *dsc_sink_color_depth_cap);
 
 static inline bool
 drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])