| Message ID | 20221109040324.17675-1-nischal.varide@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: mode clock check related to max dotclk frequency | expand |
On Wed, 09 Nov 2022, Nischal Varide <nischal.varide@intel.com> wrote: > A check on mode->clock to see if is greater than i915->max_dotclk_freq > or greater than 2 * (i915_max_dotclk_freq) in case of big-joiner and > return an -EINVAL in both the cases The commit message should explain *why* the change is being done. > > Signed-off-by: Nischal Varide <nischal.varide@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 7400d6b4c587..813f4c369dda 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -995,6 +995,10 @@ intel_dp_mode_valid(struct drm_connector *_connector, > bigjoiner = true; > max_dotclk *= 2; > } > + > + if (mode->clock > max_dotclk) > + return -EINVAL; > + The return type of this function is enum drm_mode_status, which indicates the reason for rejecting the mode. It's not a negative error code. Near the top of the function we have "target_clock = mode->clock;" making the above identical to the check we already have below. Apart from the incorrect return code. What are you trying to do? BR, Jani. > if (target_clock > max_dotclk) > return MODE_CLOCK_HIGH;
On Wed, Nov 09, 2022 at 12:36:44PM +0200, Jani Nikula wrote: > On Wed, 09 Nov 2022, Nischal Varide <nischal.varide@intel.com> wrote: > > A check on mode->clock to see if is greater than i915->max_dotclk_freq > > or greater than 2 * (i915_max_dotclk_freq) in case of big-joiner and > > return an -EINVAL in both the cases > > The commit message should explain *why* the change is being done. > > > > > Signed-off-by: Nischal Varide <nischal.varide@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 7400d6b4c587..813f4c369dda 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -995,6 +995,10 @@ intel_dp_mode_valid(struct drm_connector *_connector, > > bigjoiner = true; > > max_dotclk *= 2; > > } > > + > > + if (mode->clock > max_dotclk) > > + return -EINVAL; > > + > > The return type of this function is enum drm_mode_status, which > indicates the reason for rejecting the mode. It's not a negative error > code. > > Near the top of the function we have "target_clock = mode->clock;" > making the above identical to the check we already have below. Apart > from the incorrect return code. > > What are you trying to do? > > BR, > Jani. Yes I agree with Jani here that since target_clock is mode->clock we already have that check in place and infact returing MODE_CLOCK_HIGH makes more sense than returning just a -EINVAL What is the purpose of this change? Manasi > > > > if (target_clock > max_dotclk) > > return MODE_CLOCK_HIGH; > > > > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7400d6b4c587..813f4c369dda 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -995,6 +995,10 @@ intel_dp_mode_valid(struct drm_connector *_connector, bigjoiner = true; max_dotclk *= 2; } + + if (mode->clock > max_dotclk) + return -EINVAL; + if (target_clock > max_dotclk) return MODE_CLOCK_HIGH;
A check on mode->clock to see if is greater than i915->max_dotclk_freq or greater than 2 * (i915_max_dotclk_freq) in case of big-joiner and return an -EINVAL in both the cases Signed-off-by: Nischal Varide <nischal.varide@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++ 1 file changed, 4 insertions(+)