On Mon, 2025-12-15 at 13:53 +0200, Imre Deak wrote: > On Mon, Dec 15, 2025 at 09:46:24AM +0200, Luca Coelho wrote: > > On Thu, 2025-11-27 at 19:49 +0200, Imre Deak wrote: > > > Factor out align_max_sink_dsc_input_bpp(), also used later for computing > > > the maximum DSC input BPP limit. > > > > > > Signed-off-by: Imre Deak <[email protected]> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 28 ++++++++++++++++--------- > > > 1 file changed, 18 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 000fccc39a292..dcb9bc11e677b 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -1893,12 +1893,27 @@ int intel_dp_dsc_max_src_input_bpc(struct > > > intel_display *display) > > > return intel_dp_dsc_min_src_input_bpc(); > > > } > > > > > > +static int align_max_sink_dsc_input_bpp(const struct intel_connector > > > *connector, > > > + int max_pipe_bpp) > > > +{ > > > + u8 dsc_bpc[3]; > > > > I think it's safer to use the '= {}' we had before, because that zeroes > > the array, so in case of any stack leaks, you won't leak aleatory parts > > of the memory. In this case it's only 3 bytes, so hardly anything > > important could leak, but anyway. > > As for any other variable I don't see any reason for initializing it, if > it will be initialized before its first use. It will be initialized > before its first use by drm_dp_dsc_sink_supported_input_bpcs().
Fair enough. Security here is probably not so important, and as I said, it's only 3 bytes, but in wifi we once had the activity of pre- initializing all arrays like this for security reasons. Your call. > > Also, since this is 3 bytes long, it's theoretically better to have it > > at the end of the stack declarations. > > The compiler is free to reorder the allocation order on the stack and > is expected to that for optimal alignment. Of course the compiler will do this sort of things, but it's just better practice IMHO to keeps organized in some way. If you had said that it was in alphabetical order (it isn't), then it would probably satisfy my OCD. lol In any case, these were just nitpicks, so it's up to you. Reviewed-by: Luca Coelho <[email protected]> -- Cheers, Luca. > > > + int num_bpc; > > > + int i; > > > + > > > + num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd, > > > + dsc_bpc); > > > + for (i = 0; i < num_bpc; i++) { > > > + if (dsc_bpc[i] * 3 <= max_pipe_bpp) > > > + return dsc_bpc[i] * 3; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > int intel_dp_dsc_compute_max_bpp(const struct intel_connector *connector, > > > u8 max_req_bpc) > > > { > > > struct intel_display *display = to_intel_display(connector); > > > - int i, num_bpc; > > > - u8 dsc_bpc[3] = {}; > > > int dsc_max_bpc; > > > > > > dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display); > > > @@ -1908,14 +1923,7 @@ int intel_dp_dsc_compute_max_bpp(const struct > > > intel_connector *connector, > > > > > > dsc_max_bpc = min(dsc_max_bpc, max_req_bpc); > > > > > > - num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd, > > > - dsc_bpc); > > > - for (i = 0; i < num_bpc; i++) { > > > - if (dsc_max_bpc >= dsc_bpc[i]) > > > - return dsc_bpc[i] * 3; > > > - } > > > - > > > - return 0; > > > + return align_max_sink_dsc_input_bpp(connector, dsc_max_bpc * 3); > > > } > > > > > > static int intel_dp_source_dsc_version_minor(struct intel_display > > > *display)
