On Mon, Dec 15, 2025 at 02:02:07PM +0200, Luca Coelho wrote: > 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.
I don't see how it is more secure. I think any valid reason to zero out variables on the stack for security reasons would need to be a guideline explained and mandated in the whole kernel ubiquitously and should not be considered as an opt-in practice. I'm not aware of such a guideline. > > > 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 The ordering rule I follow is the readability of declarations, which is better if it's in decreasing line length order. > 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)
