linux-kernel - Re: [PATCH v3 1/3] drm/msm/dsi: support DSC configurations with slice_per

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABymUCNZmxzRaVVLU=U9QPupK0KpW_C1eqa8t_ijL6B5EdgnAw@mail.gmail.com>
Date: Thu, 25 Sep 2025 15:07:28 +0800
From: Jun Nie <jun.nie@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Rob Clark <robin.clark@....qualcomm.com>, Abhinav Kumar <abhinav.kumar@...ux.dev>, 
	Jessica Zhang <jessica.zhang@....qualcomm.com>, Dmitry Baryshkov <lumag@...nel.org>, 
	Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Krishna Manikandan <quic_mkrishn@...cinc.com>, linux-arm-msm@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	Jonathan Marek <jonathan@...ek.ca>
Subject: Re: [PATCH v3 1/3] drm/msm/dsi: support DSC configurations with
 slice_per_pkt > 1

Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年9月25日周四 03:10写道：
>
> On Wed, Sep 24, 2025 at 11:08:10PM +0800, Jun Nie wrote:
> > Some panels support multiple slice to be sent in a single DSC packet. And
>
> s/support/require/
>
> If the panel supports something, then we can omit that and send 1 slice
> as we currently do. If the panel requires multiple slices, it's
> mandatory.
>
> > this feature is a must for specific panels, such as JDI LPM026M648C. Add a
>
> A panel driver which executes this API is appreciated. Otherwise in a
> few months / years somebody will submit a patch 'field foo is not used
> by the kernel drivers, drop it'.

OK, will add a panel driver in next version.
>
> > dsc_slice_per_pkt member into struct mipi_dsi_device and support the
> > feature in msm mdss driver.
> >
> > Co-developed-by: Jonathan Marek <jonathan@...ek.ca>
> > Signed-off-by: Jonathan Marek <jonathan@...ek.ca>
> > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ++++++++++---------------
> >  include/drm/drm_mipi_dsi.h         |  2 ++
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index f297e3de8c3dc4e1326e70c78c046b5a19568cef..de51cb02f267205320c5a94fc4188413e05907e6 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -157,6 +157,7 @@ struct msm_dsi_host {
> >
> >       struct drm_display_mode *mode;
> >       struct drm_dsc_config *dsc;
> > +     unsigned int dsc_slice_per_pkt;
> >
> >       /* connected device info */
> >       unsigned int channel;
> > @@ -849,17 +850,10 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> >       slice_per_intf = dsc->slice_count;
> >
> >       total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
> > -     bytes_per_pkt = dsc->slice_chunk_size; /* * slice_per_pkt; */
> > +     bytes_per_pkt = dsc->slice_chunk_size * msm_host->dsc_slice_per_pkt;
> >
> >       eol_byte_num = total_bytes_per_intf % 3;
> > -
> > -     /*
> > -      * Typically, pkt_per_line = slice_per_intf * slice_per_pkt.
> > -      *
> > -      * Since the current driver only supports slice_per_pkt = 1,
> > -      * pkt_per_line will be equal to slice per intf for now.
> > -      */
> > -     pkt_per_line = slice_per_intf;
> > +     pkt_per_line = slice_per_intf / msm_host->dsc_slice_per_pkt;
> >
> >       if (is_cmd_mode) /* packet data type */
> >               reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
> > @@ -1008,12 +1002,8 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> >               else
> >                       /*
> >                        * When DSC is enabled, WC = slice_chunk_size * slice_per_pkt + 1.
> > -                      * Currently, the driver only supports default value of slice_per_pkt = 1
> > -                      *
> > -                      * TODO: Expand mipi_dsi_device struct to hold slice_per_pkt info
> > -                      *       and adjust DSC math to account for slice_per_pkt.
> >                        */
> > -                     wc = msm_host->dsc->slice_chunk_size + 1;
> > +                     wc = msm_host->dsc->slice_chunk_size * msm_host->dsc_slice_per_pkt + 1;
> >
> >               dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
> >                       DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> > @@ -1623,8 +1613,13 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
> >       msm_host->lanes = dsi->lanes;
> >       msm_host->format = dsi->format;
> >       msm_host->mode_flags = dsi->mode_flags;
> > -     if (dsi->dsc)
> > +     if (dsi->dsc) {
> >               msm_host->dsc = dsi->dsc;
> > +             msm_host->dsc_slice_per_pkt = dsi->dsc_slice_per_pkt;
> > +             /* for backwards compatibility, assume 1 if not set */
> > +             if (!msm_host->dsc_slice_per_pkt)
> > +                     msm_host->dsc_slice_per_pkt = 1;
>
> msm_host->dsc_slice_per_pkt = dsi->dsc_slice_per_pkt ?: 1;
>
> Yes, I think it's more ideomatic.

Will add the change.
>
> > +     }
> >
> >       ret = dsi_dev_attach(msm_host->pdev);
> >       if (ret)
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 6d2c08e8110151a97620389197f1ef79c058329d..5a85ba01f402a3866b70828391bb417bb8dd5956 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -182,6 +182,7 @@ struct mipi_dsi_device_info {
> >   * be set to the real limits of the hardware, zero is only accepted for
> >   * legacy drivers
> >   * @dsc: panel/bridge DSC pps payload to be sent
> > + * @dsc_slice_per_pkt: number of DSC slices to be sent as in a single packet
>
> s/as //

Will do.
>
> >   */
> >  struct mipi_dsi_device {
> >       struct mipi_dsi_host *host;
> > @@ -196,6 +197,7 @@ struct mipi_dsi_device {
> >       unsigned long hs_rate;
> >       unsigned long lp_rate;
> >       struct drm_dsc_config *dsc;
> > +     unsigned int dsc_slice_per_pkt;
>
> Why is it a part of the DSI device config? What if a device specifies
> dsc_slice_per_pkt, but not DSC config? What are the legit boundaries for
> this field?

You are right. drm_dsc_config is better place to add the info. Thus only
panels that support DSC can convey the info to host.
>
> >  };
> >
> >  /**
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry