On Tue, 19 Aug 2025, "Kandpal, Suraj" <suraj.kand...@intel.com> wrote:
>>
>> On Tue, 19 Aug 2025, Suraj Kandpal <suraj.kand...@intel.com> wrote:
>> > We need override certain link rates in favour of the next available
>> > higher link rate. The Link rates that need to be overridden are
>> > indicated by a mask in VBT. To make sure these modes are skipped we
>> > don't add them in them in the sink rates array.
>> >
>> > --v2
>> > -Update the link rates after we have a final set of link rates [Ankit]
>> > -Break this patch up [Ankit] -Optimize the assingment during loop
>> > [Ankit]
>> >
>> > --v3
>> > -Add protection against broken VBTs [Jani]
>> >
>> > --v4
>> > -Fix build errors
>> > -Create a seprate function to check if edp data override is selected
>> > and using the correct vbt
>> >
>> > --v5
>> > -Use correct number to check the num of edp rates [Ankit]
>> >
>> > Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_bios.c | 15 ++++++++++++++-
>> > drivers/gpu/drm/i915/display/intel_bios.h | 2 ++
>> > drivers/gpu/drm/i915/display/intel_dp.c | 22 ++++++++++++++++++++++
>> > 3 files changed, 38 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
>> > b/drivers/gpu/drm/i915/display/intel_bios.c
>> > index 444ed54f7c35..05a74c3bc9af 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> > @@ -2521,11 +2521,24 @@ int intel_bios_dp_max_lane_count(const struct
>> > intel_bios_encoder_data *devdata) bool
>> > intel_bios_encoder_reject_edp_rate(const struct intel_bios_encoder_data
>> *devdata,
>> > int rate)
>> > +{
>> > + return devdata->child.edp_data_rate_override &
>> > +edp_rate_override_mask(rate); }
>> > +
>> > +bool
>> > +intel_bios_vbt_supports_edp_data_override(const struct
>> > +intel_bios_encoder_data *devdata)
>>
>> Why are you adding this function? Earlier versions didn't have it, and the
>> reason
>> for its existence isn't explained or clear.
>>
>
> This function was added in v3 (I did add a small mention in the commit
> message I can maybe elaborate more there)
> in response to one of your comments in which you mentioned the
> Possibility of ending up with a broken vbt that masked all the link rates
> causing us to have no link rate
> To drive the edp with this function now checks if we have at least one edp
> link rate using which we can
> Drive edp if not we do not modify the list of sink rates at all.
You can check for that scenario in intel_bios_encoder_reject_edp_rate(),
and return false for all rates if all of the bits are set.
We don't have to leak these checks into callers.
BR,
Jani.
>
> Regards,
> Suraj Kandpal
>
>> BR,
>> Jani.
>>
>>
>> > {
>> > if (!devdata || devdata->display->vbt.version < 263)
>> > return false;
>> >
>> > - return devdata->child.edp_data_rate_override &
>> edp_rate_override_mask(rate);
>> > + /*
>> > + * This means the VBT ends up asking us to override every possible rate
>> > + * indicating the VBT is broken so skip this
>> > + */
>> > + if (hweight32(devdata->child.edp_data_rate_override) >=
>> BDB_263_VBT_EDP_NUM_RATES)
>> > + return false;
>> > +
>> > + return true;
>> > }
>> >
>> > static void sanitize_device_type(struct intel_bios_encoder_data
>> > *devdata, diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
>> > b/drivers/gpu/drm/i915/display/intel_bios.h
>> > index 781e08f7eeb2..d24660bcc7f3 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_bios.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_bios.h
>> > @@ -276,5 +276,7 @@ void intel_bios_for_each_encoder(struct intel_display
>> *display,
>> > const struct
>> intel_bios_encoder_data *devdata));
>> >
>> > void intel_bios_debugfs_register(struct intel_display *display);
>> > +bool
>> > +intel_bios_vbt_supports_edp_data_override(const struct
>> > +intel_bios_encoder_data *devdata);
>> >
>> > #endif /* _INTEL_BIOS_H_ */
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 54d88f24b689..f6fad42182ae 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -4277,6 +4277,26 @@ static void intel_edp_mso_init(struct intel_dp
>> *intel_dp)
>> > intel_dp->mso_pixel_overlap = mso ? info->mso_pixel_overlap : 0; }
>> >
>> > +static void
>> > +intel_edp_set_data_override_rates(struct intel_dp *intel_dp) {
>> > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> > + int *sink_rates = intel_dp->sink_rates;
>> > + int i, j = 0;
>> > +
>> > + if (!intel_bios_vbt_supports_edp_data_override(encoder->devdata))
>> > + return;
>> > +
>> > + for (i = 0; i < intel_dp->num_sink_rates; i++) {
>> > + if (intel_bios_encoder_reject_edp_rate(encoder->devdata,
>> > + intel_dp->sink_rates[i]))
>> > + continue;
>> > +
>> > + sink_rates[j++] = intel_dp->sink_rates[i];
>> > + }
>> > + intel_dp->num_sink_rates = j;
>> > +}
>> > +
>> > static void
>> > intel_edp_set_sink_rates(struct intel_dp *intel_dp) { @@ -4327,6
>> > +4347,8 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>> > intel_dp->use_rate_select = true;
>> > else
>> > intel_dp_set_sink_rates(intel_dp);
>> > +
>> > + intel_edp_set_data_override_rates(intel_dp);
>> > }
>> >
>> > static bool
>>
>> --
>> Jani Nikula, Intel
--
Jani Nikula, Intel
