Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Marijn Suijten
On 2022-10-06 00:11:06, Dmitry Baryshkov wrote:
> On 06/10/2022 00:08, Marijn Suijten wrote:
> > [..]
> > Aside fixing that to assign these values (through the proper constants)
> > to dsc->mux_word_size, is it worth looking for the right parameters for
> > other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
> > support for other bpc is validated?
> 
> I'd say, return -EINVAL or -EOPNOTSUPP for now, let's fix that later. 
> It's definitely a separate change. Let's wait for a device with such 
> panel to be able to test it.

According to [1] these four fields specifically are different for other
BPC; I can add a -EOPNOTSUPP and DRM_DEV_ERROR requesting a test, or
insert the values; there's only 8BPC and 10BPC, no 12BPC.

Aside that we need a different initial_offset = 5632 for other bpp/bpc
pairs.

[1]: 
https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139

- Marijn


Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov

On 06/10/2022 00:08, Marijn Suijten wrote:

On 2022-10-05 22:58:48, Marijn Suijten wrote:

On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:

[..]
In fact, could you please take a look if we can switch to using this
function and drop our code?


[..]

Do you want me to do this in a v3, before applying this fractional-bits
fix?  [..]


One thing to note:

/* bpc 8 */
dsc->flatness_min_qp = 3;
dsc->flatness_max_qp = 12;
dsc->rc_quant_incr_limit0 = 11;
dsc->rc_quant_incr_limit1 = 11;
dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;

Here a bunch of properties are hardcoded, seemingly for bpc = 8.
mux_word_size is only ever read in drm_dsc_compute_rc_parameters() so
only becomes relevant _after_ the migration, and is currently dealt with
correctly by:

mux_words_size = 48;/* bpc == 8/10 */
if (dsc->bits_per_component == 12)
mux_words_size = 64;

Aside fixing that to assign these values (through the proper constants)
to dsc->mux_word_size, is it worth looking for the right parameters for
other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
support for other bpc is validated?


I'd say, return -EINVAL or -EOPNOTSUPP for now, let's fix that later. 
It's definitely a separate change. Let's wait for a device with such 
panel to be able to test it.


--
With best wishes
Dmitry



Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Marijn Suijten
On 2022-10-05 22:58:48, Marijn Suijten wrote:
> On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:
> > [..]
> > In fact, could you please take a look if we can switch to using this
> > function and drop our code?
>
> [..]
>
> Do you want me to do this in a v3, before applying this fractional-bits
> fix?  [..]

One thing to note:

/* bpc 8 */
dsc->flatness_min_qp = 3;
dsc->flatness_max_qp = 12;
dsc->rc_quant_incr_limit0 = 11;
dsc->rc_quant_incr_limit1 = 11;
dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;

Here a bunch of properties are hardcoded, seemingly for bpc = 8.
mux_word_size is only ever read in drm_dsc_compute_rc_parameters() so
only becomes relevant _after_ the migration, and is currently dealt with
correctly by:

mux_words_size = 48;/* bpc == 8/10 */
if (dsc->bits_per_component == 12)
mux_words_size = 64;

Aside fixing that to assign these values (through the proper constants)
to dsc->mux_word_size, is it worth looking for the right parameters for
other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
support for other bpc is validated?

- Marijn


Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov

On 05/10/2022 23:58, Marijn Suijten wrote:

On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:

On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
 wrote:


drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
  1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index f42794cdd4c1..4717d49d76be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -33,7 +33,7 @@

  #define DSI_RESET_TOGGLE_DELAY_MS 20

-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc);

  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
  {
@@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
 u32 va_end = va_start + mode->vdisplay;
 u32 hdisplay = mode->hdisplay;
 u32 wc;
+   int ret;

 DBG("");

@@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
 /* we do the calculations for dsc parameters here so that
  * panel can use these parameters
  */
-   dsi_populate_dsc_params(dsc);
+   ret = dsi_populate_dsc_params(msm_host, dsc);
+   if (ret)
+   return;

 /* Divide the display by 3 but keep back/font porch and
  * pulse width same
@@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
  };

-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc)
  {
 int mux_words_size;
 int groups_per_line, groups_total;
@@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 int data;
 int final_value, final_scale;
 int i;
+   u16 bpp = dsc->bits_per_pixel >> 4;
+
+   if (dsc->bits_per_pixel & 0xf) {
+   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support fractional 
bits_per_pixel\n");
+   return -EINVAL;
+   }

 dsc->rc_model_size = 8192;
 dsc->first_line_bpg_offset = 12;
@@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 }

 dsc->initial_offset = 6144; /* Not bpp 12 */
-   if (dsc->bits_per_pixel != 8)
+   if (bpp != 8)
 dsc->initial_offset = 2048; /* bpp = 12 */

 mux_words_size = 48;/* bpc == 8/10 */
@@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
drm_dsc_config *dsc)
  * params are calculated
  */
 groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);


I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
The mentioned function has the following code:

vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *

vdsc_cfg->bits_per_pixel,
   (8 * 16));


Fwiw this discussion happened in dsi_update_dsc_timing() where a similar
calculation was the sole user of bits_per_pixel.  This function,
dsi_populate_dsc_params(), has more uses of bits_per_pixel so it made
more sense to compute and document this "discrepancy" up front.
drm_dsc_compute_rc_parameters() doesn't document where this "16" comes
from, unfortunately (requiring knowledge of the contents of the struct).


In fact, could you please take a look if we can switch to using this
function and drop our code?


There's alread a:

/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
 * params are calculated
 */

And it was trivial to replace everything below that comment with this
function call; I have not checked the math in detail but it assigns
_every_ field that was also assigned here, and the resulting values
provide an identical DCS PPS (which I happened to be printing to compare
to downstream, leading to find all the issues solved in this series) and
working phone sc

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Marijn Suijten
On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:
> On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
>  wrote:
> >
> > drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> > bits, which all panel drivers should adhere to for
> > drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
> > DSI driver here seems to assume that this field doesn't contain any
> > fractional bits, hence resulting in the wrong values being computed.
> > Since none of the calculations leave any room for fractional bits or
> > seem to indicate any possible area of support, disallow such values
> > altogether.
> >
> > Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> > Signed-off-by: Marijn Suijten 
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index f42794cdd4c1..4717d49d76be 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -33,7 +33,7 @@
> >
> >  #define DSI_RESET_TOGGLE_DELAY_MS 20
> >
> > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
> > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> > drm_dsc_config *dsc);
> >
> >  static int dsi_get_version(const void __iomem *base, u32 *major, u32 
> > *minor)
> >  {
> > @@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host 
> > *msm_host, bool is_bonded_dsi)
> > u32 va_end = va_start + mode->vdisplay;
> > u32 hdisplay = mode->hdisplay;
> > u32 wc;
> > +   int ret;
> >
> > DBG("");
> >
> > @@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host 
> > *msm_host, bool is_bonded_dsi)
> > /* we do the calculations for dsc parameters here so that
> >  * panel can use these parameters
> >  */
> > -   dsi_populate_dsc_params(dsc);
> > +   ret = dsi_populate_dsc_params(msm_host, dsc);
> > +   if (ret)
> > +   return;
> >
> > /* Divide the display by 3 but keep back/font porch and
> >  * pulse width same
> > @@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> > 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
> >  };
> >
> > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> > drm_dsc_config *dsc)
> >  {
> > int mux_words_size;
> > int groups_per_line, groups_total;
> > @@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> > int data;
> > int final_value, final_scale;
> > int i;
> > +   u16 bpp = dsc->bits_per_pixel >> 4;
> > +
> > +   if (dsc->bits_per_pixel & 0xf) {
> > +   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
> > fractional bits_per_pixel\n");
> > +   return -EINVAL;
> > +   }
> >
> > dsc->rc_model_size = 8192;
> > dsc->first_line_bpg_offset = 12;
> > @@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> > }
> >
> > dsc->initial_offset = 6144; /* Not bpp 12 */
> > -   if (dsc->bits_per_pixel != 8)
> > +   if (bpp != 8)
> > dsc->initial_offset = 2048; /* bpp = 12 */
> >
> > mux_words_size = 48;/* bpc == 8/10 */
> > @@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> >  * params are calculated
> >  */
> > groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> > -   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
> > dsc->bits_per_pixel, 8);
> > +   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
> 
> I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
> The mentioned function has the following code:
> 
> vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
> 
> vdsc_cfg->bits_per_pixel,
>   (8 * 16));

Fwiw this discussion happened in dsi_update_dsc_timing() where a similar
calculation was the sole user of bits_per_pixel.  This function,
dsi_populate_dsc_params(), has more uses of bits_per_pixel so it made
more sense to compute and document this "discrepancy" up front.
drm_dsc_compute_rc_parameters() doesn't document where this "16" comes
from, unfortunately (requiring knowledge of the contents of the struct).

> In fact, could you please take a look if we can switch to using this
> function and drop our code?

There's alread a:

/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
 * params are calculated
 */

And it was trivial to replace everything below that com

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov
On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
 wrote:
>
> drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> bits, which all panel drivers should adhere to for
> drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
> DSI driver here seems to assume that this field doesn't contain any
> fractional bits, hence resulting in the wrong values being computed.
> Since none of the calculations leave any room for fractional bits or
> seem to indicate any possible area of support, disallow such values
> altogether.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten 
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index f42794cdd4c1..4717d49d76be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -33,7 +33,7 @@
>
>  #define DSI_RESET_TOGGLE_DELAY_MS 20
>
> -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
> +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> drm_dsc_config *dsc);
>
>  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
>  {
> @@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
> u32 va_end = va_start + mode->vdisplay;
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> +   int ret;
>
> DBG("");
>
> @@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
> /* we do the calculations for dsc parameters here so that
>  * panel can use these parameters
>  */
> -   dsi_populate_dsc_params(dsc);
> +   ret = dsi_populate_dsc_params(msm_host, dsc);
> +   if (ret)
> +   return;
>
> /* Divide the display by 3 but keep back/font porch and
>  * pulse width same
> @@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
>  };
>
> -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> drm_dsc_config *dsc)
>  {
> int mux_words_size;
> int groups_per_line, groups_total;
> @@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> int data;
> int final_value, final_scale;
> int i;
> +   u16 bpp = dsc->bits_per_pixel >> 4;
> +
> +   if (dsc->bits_per_pixel & 0xf) {
> +   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
> fractional bits_per_pixel\n");
> +   return -EINVAL;
> +   }
>
> dsc->rc_model_size = 8192;
> dsc->first_line_bpg_offset = 12;
> @@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> }
>
> dsc->initial_offset = 6144; /* Not bpp 12 */
> -   if (dsc->bits_per_pixel != 8)
> +   if (bpp != 8)
> dsc->initial_offset = 2048; /* bpp = 12 */
>
> mux_words_size = 48;/* bpc == 8/10 */
> @@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
>  * params are calculated
>  */
> groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> -   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
> dsc->bits_per_pixel, 8);
> +   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);

I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
The mentioned function has the following code:

vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *

vdsc_cfg->bits_per_pixel,
  (8 * 16));

In fact, could you please take a look if we can switch to using this
function and drop our code?

>
> /* rbs-min */
> min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
> -   dsc->initial_xmit_delay * dsc->bits_per_pixel 
> +
> +   dsc->initial_xmit_delay * bpp +
> groups_per_line * dsc->first_line_bpg_offset;
>
> -   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
> +   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
>
> dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
>
> @@ -1854,7 +1863,7 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
> num_extra_mux_bits);
> dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
>
> -   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
> +   dat

[PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Marijn Suijten
drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index f42794cdd4c1..4717d49d76be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -33,7 +33,7 @@
 
 #define DSI_RESET_TOGGLE_DELAY_MS 20
 
-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc);
 
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
 {
@@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
u32 va_end = va_start + mode->vdisplay;
u32 hdisplay = mode->hdisplay;
u32 wc;
+   int ret;
 
DBG("");
 
@@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
/* we do the calculations for dsc parameters here so that
 * panel can use these parameters
 */
-   dsi_populate_dsc_params(dsc);
+   ret = dsi_populate_dsc_params(msm_host, dsc);
+   if (ret)
+   return;
 
/* Divide the display by 3 but keep back/font porch and
 * pulse width same
@@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
 };
 
-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc)
 {
int mux_words_size;
int groups_per_line, groups_total;
@@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
int data;
int final_value, final_scale;
int i;
+   u16 bpp = dsc->bits_per_pixel >> 4;
+
+   if (dsc->bits_per_pixel & 0xf) {
+   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
fractional bits_per_pixel\n");
+   return -EINVAL;
+   }
 
dsc->rc_model_size = 8192;
dsc->first_line_bpg_offset = 12;
@@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
}
 
dsc->initial_offset = 6144; /* Not bpp 12 */
-   if (dsc->bits_per_pixel != 8)
+   if (bpp != 8)
dsc->initial_offset = 2048; /* bpp = 12 */
 
mux_words_size = 48;/* bpc == 8/10 */
@@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
drm_dsc_config *dsc)
 * params are calculated
 */
groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
 
/* rbs-min */
min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
-   dsc->initial_xmit_delay * dsc->bits_per_pixel +
+   dsc->initial_xmit_delay * bpp +
groups_per_line * dsc->first_line_bpg_offset;
 
-   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
+   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
 
dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
 
@@ -1854,7 +1863,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
num_extra_mux_bits);
dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
 
-   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
+   data = dsc->initial_xmit_delay * bpp;
final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
dsc->final_offset = final_value;
 
-- 
2.38.0