Re: [PATCH v5 2/6] drm/dp: Add helpers to query the branch DSC max throughput/line-width

[Imre Deak <imre.deak@intel.com>]

On Tue, Sep 30, 2025 at 11:04:47PM +0300, Dmitry Baryshkov wrote:
> On Tue, Sep 30, 2025 at 02:38:09PM +0300, Imre Deak wrote:
> > On Tue, Sep 30, 2025 at 08:30:10AM +0300, Dmitry Baryshkov wrote:
> > > On Mon, Sep 29, 2025 at 01:10:17PM +0300, Imre Deak wrote:
> > > > On Mon, Sep 29, 2025 at 12:00:03PM +0300, Dmitry Baryshkov wrote:
> > > > > On Mon, Sep 29, 2025 at 09:36:44AM +0300, Imre Deak wrote:
> > > > > > Add helpers to query the DP DSC sink device's per-slice throughput as
> > > > > > well as a DSC branch device's overall throughput and line-width
> > > > > > capabilities.
> > > > > > 
> > > > > > v2 (Ville):
> > > > > > - Rename pixel_clock to peak_pixel_rate, document what the value means
> > > > > >   in case of MST tiled displays.
> > > > > > - Fix name of drm_dp_dsc_branch_max_slice_throughput() to
> > > > > >   drm_dp_dsc_sink_max_slice_throughput().
> > > > > > v3:
> > > > > > - Fix the DSC branch device minimum valid line width value from 2560
> > > > > >   to 5120 pixels.
> > > > > > - Fix drm_dp_dsc_sink_max_slice_throughput()'s pixel_clock parameter
> > > > > >   name to peak_pixel_rate in header file.
> > > > > > - Add handling for throughput mode 0 granular delta, defined by DP
> > > > > >   Standard v2.1a.
> > > > > 
> > > > > This one got sent as a separate V5, without a proper changelog. What has
> > > > > changed?
> > > > 
> > > > This is v3 of the patch, the changes are listed under v3. The patchset's
> > > > version is v5.
> > >
> > > Ugh. How one does relate this v3 (which is not mentioned anywhere) and
> > > v5 of the series? This is totally counterintuitive. A usual
> > > recommendation is to send the full series and to send it as a new
> > > thread, sending all the patches in one go.
> > 
> > It's a common practice on intel-gfx to send a new version of one patch
> > on top of the last patchset version in that patchset's thread. For
> 
> I don't want to step on intel-gfx ways of working, but how would that
> work with e.g. 'b4 shazam'?

I don't - yet - use b4, but I suppose it must work because CI/patchwork
uses it and it does manage to assemble the new patchset for its test run
based on the In-reply-to header I think.

> > matching the patch version to the patchset version I can change the
> > patch version log above to be like:
> > 
> > v2 (Ville):
> > - Rename pixel_clock to peak_pixel_rate ...
> > v3-v4:
> > - No changes
> > v5:
> > - Fix the DSC branch device minimum valid line width value ...
> 
> Yes, I think that's more obvious, thanks!

I've been brooding over how to send the next version and then decided to
stick with the current practice and sent it as [1]. That is, increment
the patchset version to the next version of the patchset which is 6 and
increment the version of the patch with a change to the next version of
the patch itself which is 4.

Not sure if introducing the above "No changes" revisions is a good idea
after all. When the patchset is merged, this notation would lose its
meaning: you see then only the individual patches, not patchsets, so the
only interesting thing in that case is the versions the patch itself
went through. Keeping the version of the patchset and the patches within
it in sync could be also too tedious, especially if you wanted the
version of a patch without any changes to be brought up to the patchset
version.

One view is that the version of the patchset is simply different than
that of the patches within it. If an individual patch is sent in-reply
to a previous patchset, then it should be clear that this patch has a
change on top of the previous patchset and the changes are described
under the last revision log of the patch. Otherwise, when the whole
patchset is resent, the changes in that patchset version must be anyway
described in the cover letter.

I'm open to ideas, but this is my stance atm, also based on past
discussions about it with Ville and Jani.

--Imre

[1] https://lore.kernel.org/all/20250930182450.563016-1-imre.deak@intel.com

> > > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/display/drm_dp_helper.c | 156 ++++++++++++++++++++++++
> > > > > >  include/drm/display/drm_dp.h            |   3 +
> > > > > >  include/drm/display/drm_dp_helper.h     |   5 +
> > > > > >  3 files changed, 164 insertions(+)
> > > > > > 
> > > > > 
> > > > > -- 
> > > > > With best wishes
> > > > > Dmitry
> > > 
> > > -- 
> > > With best wishes
> > > Dmitry
> 
> -- 
> With best wishes
> Dmitry