Currently, when compression is enabled, hdisplay is reduced via integer
division. This causes issues for modes where the original hdisplay is
not a multiple of 3.
To fix this, use DIV_ROUND_UP to divide hdisplay.
Reviewed-by: Marijn Suijten <[email protected]>
Suggested-by: Marijn Suijten <[email protected]>
Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Jessica Zhang <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 9223d7ec5a73..18d38b90eb28 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
* pulse width same
*/
h_total -= hdisplay;
- hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
+ hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
h_total += hdisplay;
ha_end = ha_start + hdisplay;
}
--
2.40.1
On 22.05.2023 22:44, Marijn Suijten wrote:
> On 2023-05-22 13:30:20, Jessica Zhang wrote:
>> Currently, when compression is enabled, hdisplay is reduced via integer
>> division. This causes issues for modes where the original hdisplay is
>> not a multiple of 3.
>>
>> To fix this, use DIV_ROUND_UP to divide hdisplay.
>>
>> Reviewed-by: Marijn Suijten <[email protected]>
>> Suggested-by: Marijn Suijten <[email protected]>
>
> Nit: probably these should go in the opposite order. And if they're
> all supposed to be chronological, I think it is:
>
> Suggested-by:
> Fixes:
> Signed-off-by:
> Reviewed-by:
>
> But unsure if that's a hard requirement, or even correct at all.
>
> - Marijn
Or you can rely on b4 to pick that up if it comes from others
Konrad
>
>> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
>> Signed-off-by: Jessica Zhang <[email protected]>
>> ---
>> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index 9223d7ec5a73..18d38b90eb28 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>> * pulse width same
>> */
>> h_total -= hdisplay;
>> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
>> + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>> h_total += hdisplay;
>> ha_end = ha_start + hdisplay;
>> }
>>
>> --
>> 2.40.1
>>
On 2023-05-22 13:30:20, Jessica Zhang wrote:
> Currently, when compression is enabled, hdisplay is reduced via integer
> division. This causes issues for modes where the original hdisplay is
> not a multiple of 3.
>
> To fix this, use DIV_ROUND_UP to divide hdisplay.
>
> Reviewed-by: Marijn Suijten <[email protected]>
> Suggested-by: Marijn Suijten <[email protected]>
Nit: probably these should go in the opposite order. And if they're
all supposed to be chronological, I think it is:
Suggested-by:
Fixes:
Signed-off-by:
Reviewed-by:
But unsure if that's a hard requirement, or even correct at all.
- Marijn
> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
> Signed-off-by: Jessica Zhang <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 9223d7ec5a73..18d38b90eb28 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> * pulse width same
> */
> h_total -= hdisplay;
> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
> + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> h_total += hdisplay;
> ha_end = ha_start + hdisplay;
> }
>
> --
> 2.40.1
>
On 2023-05-22 22:52:40, Konrad Dybcio wrote:
>
>
> On 22.05.2023 22:44, Marijn Suijten wrote:
> > On 2023-05-22 13:30:20, Jessica Zhang wrote:
> >> Currently, when compression is enabled, hdisplay is reduced via integer
> >> division. This causes issues for modes where the original hdisplay is
> >> not a multiple of 3.
> >>
> >> To fix this, use DIV_ROUND_UP to divide hdisplay.
> >>
> >> Reviewed-by: Marijn Suijten <[email protected]>
> >> Suggested-by: Marijn Suijten <[email protected]>
> >
> > Nit: probably these should go in the opposite order. And if they're
> > all supposed to be chronological, I think it is:
> >
> > Suggested-by:
> > Fixes:
> > Signed-off-by:
> > Reviewed-by:
> >
> > But unsure if that's a hard requirement, or even correct at all.
> >
> > - Marijn
> Or you can rely on b4 to pick that up if it comes from others
The problem is that somewhat stupidly, b4 (trailers -u) puts them in the
wrong (not chronological) order, so it's pretty much useless for this.
Unless there's a required ordering specified somewhere in the docs, that
is *not* chronological, and that b4 is abiding by? (that is my question
above)
- Marijn
>
> Konrad
<snip>
On 5/22/2023 1:44 PM, Marijn Suijten wrote:
> On 2023-05-22 13:30:20, Jessica Zhang wrote:
>> Currently, when compression is enabled, hdisplay is reduced via integer
>> division. This causes issues for modes where the original hdisplay is
>> not a multiple of 3.
>>
>> To fix this, use DIV_ROUND_UP to divide hdisplay.
>>
>> Reviewed-by: Marijn Suijten <[email protected]>
>> Suggested-by: Marijn Suijten <[email protected]>
>
> Nit: probably these should go in the opposite order. And if they're
> all supposed to be chronological, I think it is:
>
> Suggested-by:
> Fixes:
> Signed-off-by:
> Reviewed-by:
>
> But unsure if that's a hard requirement, or even correct at all.
Hi Marijn,
I don't see any explicit documentation on the order of R-b tags. FWIW, I
see in the git log that S-o-b always goes at the bottom of the commit
message.
I would prefer the S-o-b to always be at the bottom (as it helps me
avoid duplicate S-o-b's when doing `git commit -s`), though I can flip
the order of the R-b and suggested-by tags.
Thanks,
Jessica Zhang
>
> - Marijn
>
>> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
>> Signed-off-by: Jessica Zhang <[email protected]>
>> ---
>> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index 9223d7ec5a73..18d38b90eb28 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>> * pulse width same
>> */
>> h_total -= hdisplay;
>> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
>> + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>> h_total += hdisplay;
>> ha_end = ha_start + hdisplay;
>> }
>>
>> --
>> 2.40.1
>>
On 2023-05-23 01:14:40, Dmitry Baryshkov wrote:
> On Tue, 23 May 2023 at 00:45, Jessica Zhang <[email protected]> wrote:
> >
> >
> >
> > On 5/22/2023 1:44 PM, Marijn Suijten wrote:
> > > On 2023-05-22 13:30:20, Jessica Zhang wrote:
> > >> Currently, when compression is enabled, hdisplay is reduced via integer
> > >> division. This causes issues for modes where the original hdisplay is
> > >> not a multiple of 3.
> > >>
> > >> To fix this, use DIV_ROUND_UP to divide hdisplay.
> > >>
> > >> Reviewed-by: Marijn Suijten <[email protected]>
> > >> Suggested-by: Marijn Suijten <[email protected]>
> > >
> > > Nit: probably these should go in the opposite order. And if they're
> > > all supposed to be chronological, I think it is:
> > >
> > > Suggested-by:
> > > Fixes:
> > > Signed-off-by:
> > > Reviewed-by:
> > >
> > > But unsure if that's a hard requirement, or even correct at all.
> >
> > Hi Marijn,
> >
> > I don't see any explicit documentation on the order of R-b tags. FWIW, I
> > see in the git log that S-o-b always goes at the bottom of the commit
> > message.
> >
> > I would prefer the S-o-b to always be at the bottom (as it helps me
> > avoid duplicate S-o-b's when doing `git commit -s`), though I can flip
> > the order of the R-b and suggested-by tags.
>
> I'd second Jessica here. Consider these tags as a history or a transcript:
>
> I would not vote on the particular order of the Suggested-by/Fixes
> tags, I don't think that is important. These come first. Then the
> patch goes through different cycles. of reviews, which gain
> Reviewed-by tags.
>
> In the same way Link/Patchwork/whatever other tags are added in the
> historical order.
>
> By having the submitter's S-o-b at the bottom, the submitter adds the
> final signature under everything else being stated/recorded.
Correct, so the s-o-b can always be kept / moved back to the bottom on a
resend, stating that they sign off on "all that was written previously"
including picking up reviews.
However, for the rest of your reply about "history / transcript", you
seem to agree exactly with my point of keeping (or rather, simply
appending) these in chronological order?
- Marijn
>
> Of course, in a more complicated story, there might be other
> developers taking part (Co-Developed-By + Signed-off-by), etc.
>
> Note: all described is just my perception and might differ from the
> BCP regarding the tags.
<snip>
On Tue, 23 May 2023 at 00:45, Jessica Zhang <[email protected]> wrote:
>
>
>
> On 5/22/2023 1:44 PM, Marijn Suijten wrote:
> > On 2023-05-22 13:30:20, Jessica Zhang wrote:
> >> Currently, when compression is enabled, hdisplay is reduced via integer
> >> division. This causes issues for modes where the original hdisplay is
> >> not a multiple of 3.
> >>
> >> To fix this, use DIV_ROUND_UP to divide hdisplay.
> >>
> >> Reviewed-by: Marijn Suijten <[email protected]>
> >> Suggested-by: Marijn Suijten <[email protected]>
> >
> > Nit: probably these should go in the opposite order. And if they're
> > all supposed to be chronological, I think it is:
> >
> > Suggested-by:
> > Fixes:
> > Signed-off-by:
> > Reviewed-by:
> >
> > But unsure if that's a hard requirement, or even correct at all.
>
> Hi Marijn,
>
> I don't see any explicit documentation on the order of R-b tags. FWIW, I
> see in the git log that S-o-b always goes at the bottom of the commit
> message.
>
> I would prefer the S-o-b to always be at the bottom (as it helps me
> avoid duplicate S-o-b's when doing `git commit -s`), though I can flip
> the order of the R-b and suggested-by tags.
I'd second Jessica here. Consider these tags as a history or a transcript:
I would not vote on the particular order of the Suggested-by/Fixes
tags, I don't think that is important. These come first. Then the
patch goes through different cycles. of reviews, which gain
Reviewed-by tags.
In the same way Link/Patchwork/whatever other tags are added in the
historical order.
By having the submitter's S-o-b at the bottom, the submitter adds the
final signature under everything else being stated/recorded.
Of course, in a more complicated story, there might be other
developers taking part (Co-Developed-By + Signed-off-by), etc.
Note: all described is just my perception and might differ from the
BCP regarding the tags.
>
> Thanks,
>
> Jessica Zhang
>
> >
> > - Marijn
> >
> >> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
> >> Signed-off-by: Jessica Zhang <[email protected]>
> >> ---
> >> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> index 9223d7ec5a73..18d38b90eb28 100644
> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> >> * pulse width same
> >> */
> >> h_total -= hdisplay;
> >> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
> >> + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> >> h_total += hdisplay;
> >> ha_end = ha_start + hdisplay;
> >> }
> >>
> >> --
> >> 2.40.1
> >>
--
With best wishes
Dmitry
On 23/05/2023 01:18, Marijn Suijten wrote:
> On 2023-05-23 01:14:40, Dmitry Baryshkov wrote:
>> On Tue, 23 May 2023 at 00:45, Jessica Zhang <[email protected]> wrote:
>>>
>>>
>>>
>>> On 5/22/2023 1:44 PM, Marijn Suijten wrote:
>>>> On 2023-05-22 13:30:20, Jessica Zhang wrote:
>>>>> Currently, when compression is enabled, hdisplay is reduced via integer
>>>>> division. This causes issues for modes where the original hdisplay is
>>>>> not a multiple of 3.
>>>>>
>>>>> To fix this, use DIV_ROUND_UP to divide hdisplay.
>>>>>
>>>>> Reviewed-by: Marijn Suijten <[email protected]>
>>>>> Suggested-by: Marijn Suijten <[email protected]>
>>>>
>>>> Nit: probably these should go in the opposite order. And if they're
>>>> all supposed to be chronological, I think it is:
>>>>
>>>> Suggested-by:
>>>> Fixes:
>>>> Signed-off-by:
>>>> Reviewed-by:
>>>>
>>>> But unsure if that's a hard requirement, or even correct at all.
>>>
>>> Hi Marijn,
>>>
>>> I don't see any explicit documentation on the order of R-b tags. FWIW, I
>>> see in the git log that S-o-b always goes at the bottom of the commit
>>> message.
>>>
>>> I would prefer the S-o-b to always be at the bottom (as it helps me
>>> avoid duplicate S-o-b's when doing `git commit -s`), though I can flip
>>> the order of the R-b and suggested-by tags.
>>
>> I'd second Jessica here. Consider these tags as a history or a transcript:
>>
>> I would not vote on the particular order of the Suggested-by/Fixes
>> tags, I don't think that is important. These come first. Then the
>> patch goes through different cycles. of reviews, which gain
>> Reviewed-by tags.
>>
>> In the same way Link/Patchwork/whatever other tags are added in the
>> historical order.
>>
>> By having the submitter's S-o-b at the bottom, the submitter adds the
>> final signature under everything else being stated/recorded.
>
> Correct, so the s-o-b can always be kept / moved back to the bottom on a
> resend, stating that they sign off on "all that was written previously"
> including picking up reviews.
>
> However, for the rest of your reply about "history / transcript", you
> seem to agree exactly with my point of keeping (or rather, simply
> appending) these in chronological order?
Yes.
>
> - Marijn
>
>>
>> Of course, in a more complicated story, there might be other
>> developers taking part (Co-Developed-By + Signed-off-by), etc.
>>
>> Note: all described is just my perception and might differ from the
>> BCP regarding the tags.
>
> <snip>
--
With best wishes
Dmitry
On 5/22/2023 2:45 PM, Jessica Zhang wrote:
>
>
> On 5/22/2023 1:44 PM, Marijn Suijten wrote:
>> On 2023-05-22 13:30:20, Jessica Zhang wrote:
>>> Currently, when compression is enabled, hdisplay is reduced via integer
>>> division. This causes issues for modes where the original hdisplay is
>>> not a multiple of 3.
>>>
>>> To fix this, use DIV_ROUND_UP to divide hdisplay.
>>>
>>> Reviewed-by: Marijn Suijten <[email protected]>
>>> Suggested-by: Marijn Suijten <[email protected]>
>>
>> Nit: probably these should go in the opposite order. And if they're
>> all supposed to be chronological, I think it is:
>>
>> Suggested-by:
>> Fixes:
>> Signed-off-by:
>> Reviewed-by:
>>
>> But unsure if that's a hard requirement, or even correct at all.
>
> Hi Marijn,
>
> I don't see any explicit documentation on the order of R-b tags. FWIW, I
> see in the git log that S-o-b always goes at the bottom of the commit
> message.
>
> I would prefer the S-o-b to always be at the bottom (as it helps me
> avoid duplicate S-o-b's when doing `git commit -s`), though I can flip
> the order of the R-b and suggested-by tags.
Correction -- I can reorder the tags so that it's:
Suggested-by:
Fixes:
Reviewed-by:
Signed-off-by:
Thanks,
Jessica Zhang
>
> Thanks,
>
> Jessica Zhang
>
>>
>> - Marijn
>>
>>> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")
>>> Signed-off-by: Jessica Zhang <[email protected]>
>>> ---
>>> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> b/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> index 9223d7ec5a73..18d38b90eb28 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host
>>> *msm_host, bool is_bonded_dsi)
>>> * pulse width same
>>> */
>>> h_total -= hdisplay;
>>> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
>>> + hdisplay =
>>> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>>> h_total += hdisplay;
>>> ha_end = ha_start + hdisplay;
>>> }
>>>
>>> --
>>> 2.40.1
>>>