Re: [PATCH 2/7] drm/i915: Extract intel_dp_has_dsc()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 21 May 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, May 21, 2024 at 12:51:03PM +0300, Jani Nikula wrote:
>> On Mon, 20 May 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Mon, May 20, 2024 at 01:47:34PM +0300, Jani Nikula wrote:
>> >> On Fri, 17 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> >
>> >> > Extract a helper to check whether the source+sink combo
>> >> > supports DSC. That basic check is needed both during mode
>> >> > validation and compute config. We'll also need to add extra
>> >> > checks to both places, so having a single place for it is nicer.
>> >> >
>> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++++++++++--
>> >> >  1 file changed, 14 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> > index 1e88449fe5f2..7bf283b4df7f 100644
>> >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> > @@ -1220,6 +1220,19 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
>> >> >  	       connector->force_bigjoiner_enable;
>> >> >  }
>> >> >  
>> >> > +static bool intel_dp_has_dsc(struct intel_connector *connector)
>> >> 
>> >> Why not const?
>> >
>> > We've generally not consted these things. And then whenver add
>> > one const somewhere it usually ends up getting in the way later,
>> > not because we need mutability but simply because we want to
>> > call something that doesn't have the const.
>> >
>> > I suppose if we do want to start consting things more we should
>> > just do some kind of bigger pass over the whole codebase so that
>> > that there's less chance of pain later.
>> >
>> > We're also not using container_of_const() for these right now,
>> > so the const can vanish semi-accidentally when casting things.
>> >
>> > I suppose this thing might be low level enough that the const
>> > could be kept. I'll have another think about it.
>> 
>> It's just that this series drops a bunch of const because of this, which
>> feels like the opposite of what you usually do. :)
>
> I suppose.
>
> My current rule of thumb is:
> - atomic object states and fbs should be const if possible
> - everything else is not
>
> I wouldn't mind making more things const, but I suspect
> there are several sizeable rabbit holes that need to be
> dug out beforehand.

Fair enough. Like I said, the series is R-b.

J.



-- 
Jani Nikula, Intel