| Message ID | 20231114225857.19702-5-jonathan@marek.ca (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm: DSI DSC video mode fixes | expand |
On Wed, 15 Nov 2023 at 01:00, Jonathan Marek <jonathan@marek.ca> wrote: > > Make it clear why the pkt_per_line value is being "divided by 2". > > Signed-off-by: Jonathan Marek <jonathan@marek.ca> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 66f198e21a7e..842765063b1b 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -877,6 +877,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod > /* DSI_VIDEO_COMPRESSION_MODE & DSI_COMMAND_COMPRESSION_MODE > * registers have similar offsets, so for below common code use > * DSI_VIDEO_COMPRESSION_MODE_XXXX for setting bits > + * > + * pkt_per_line is log2 encoded, >>1 works for supported values (1,2,4) > */ > reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_PKT_PER_LINE(pkt_per_line >> 1); Should we switch to ffs() or fls() instead? > reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_EOL_BYTE_NUM(eol_byte_num); > -- > 2.26.1 >
On 11/15/23 2:38 AM, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 01:00, Jonathan Marek <jonathan@marek.ca> wrote: >> >> Make it clear why the pkt_per_line value is being "divided by 2". >> >> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >> --- >> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index 66f198e21a7e..842765063b1b 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -877,6 +877,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod >> /* DSI_VIDEO_COMPRESSION_MODE & DSI_COMMAND_COMPRESSION_MODE >> * registers have similar offsets, so for below common code use >> * DSI_VIDEO_COMPRESSION_MODE_XXXX for setting bits >> + * >> + * pkt_per_line is log2 encoded, >>1 works for supported values (1,2,4) >> */ >> reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_PKT_PER_LINE(pkt_per_line >> 1); > > Should we switch to ffs() or fls() instead? > Just a ffs() on its own can be confusing as well (without the information that only powers of two are possible), I think like this is better. >> reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_EOL_BYTE_NUM(eol_byte_num); >> -- >> 2.26.1 >> > >
On 16/11/2023 20:45, Jonathan Marek wrote: > On 11/15/23 2:38 AM, Dmitry Baryshkov wrote: >> On Wed, 15 Nov 2023 at 01:00, Jonathan Marek <jonathan@marek.ca> wrote: >>> >>> Make it clear why the pkt_per_line value is being "divided by 2". >>> >>> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >>> --- >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c >>> b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> index 66f198e21a7e..842765063b1b 100644 >>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> @@ -877,6 +877,8 @@ static void dsi_update_dsc_timing(struct >>> msm_dsi_host *msm_host, bool is_cmd_mod >>> /* DSI_VIDEO_COMPRESSION_MODE & DSI_COMMAND_COMPRESSION_MODE >>> * registers have similar offsets, so for below common code use >>> * DSI_VIDEO_COMPRESSION_MODE_XXXX for setting bits >>> + * >>> + * pkt_per_line is log2 encoded, >>1 works for supported >>> values (1,2,4) >>> */ >>> reg |= >>> DSI_VIDEO_COMPRESSION_MODE_CTRL_PKT_PER_LINE(pkt_per_line >> 1); >> >> Should we switch to ffs() or fls() instead? >> > > Just a ffs() on its own can be confusing as well (without the > information that only powers of two are possible), I think like this is > better. Sounds fair. Could you please then add `if (pkt_per_line > 4) drm_warn("pkt_per_line too big");` With that in place: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>> reg |= >>> DSI_VIDEO_COMPRESSION_MODE_CTRL_EOL_BYTE_NUM(eol_byte_num); >>> -- >>> 2.26.1 >>> >> >>
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 66f198e21a7e..842765063b1b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -877,6 +877,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod /* DSI_VIDEO_COMPRESSION_MODE & DSI_COMMAND_COMPRESSION_MODE * registers have similar offsets, so for below common code use * DSI_VIDEO_COMPRESSION_MODE_XXXX for setting bits + * + * pkt_per_line is log2 encoded, >>1 works for supported values (1,2,4) */ reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_PKT_PER_LINE(pkt_per_line >> 1); reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_EOL_BYTE_NUM(eol_byte_num);
Make it clear why the pkt_per_line value is being "divided by 2". Signed-off-by: Jonathan Marek <jonathan@marek.ca> --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ 1 file changed, 2 insertions(+)