| Message ID | 20190321132446.22394-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915: Refactor EDID fixed mode search | expand |
Quoting Ville Syrjala (2019-03-21 13:24:45) > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Utilize drm_mode_match() instead of hand rolling it when > looking for the DRRS downclock mode. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_panel.c | 49 +++++++++++++++--------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index cf111eba1a93..4727e74f7437 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > drm_mode_set_crtcinfo(adjusted_mode, 0); > } > > +static bool is_downclock_mode(const struct drm_display_mode *downclock_mode, > + const struct drm_display_mode *fixed_mode) > +{ > + return drm_mode_match(downclock_mode, fixed_mode, > + DRM_MODE_MATCH_TIMINGS | Adds m1->vscan == m2->vscan Makes sense. > + DRM_MODE_MATCH_FLAGS | > + DRM_MODE_MATCH_3D_FLAGS) && Together adds m1->flags == m2->flags And also makes sense. The other available flag is DRM_MODE_MATCH_ASPECT_RATIO, which is redundant given the timings match. > + downclock_mode->clock < fixed_mode->clock; > +} > + > /** > * intel_find_panel_downclock - find the reduced downclock for LVDS in EDID > * @dev_priv: i915 device instance > @@ -60,11 +70,8 @@ intel_find_panel_downclock(struct drm_i915_private *dev_priv, > struct drm_display_mode *fixed_mode, > struct drm_connector *connector) > { > - struct drm_display_mode *scan, *tmp_mode; > - int temp_downclock; > - > - temp_downclock = fixed_mode->clock; > - tmp_mode = NULL; > + const struct drm_display_mode *scan, *best_mode = NULL; > + int best_clock = fixed_mode->clock; > > list_for_each_entry(scan, &connector->probed_modes, head) { > /* > @@ -74,29 +81,21 @@ intel_find_panel_downclock(struct drm_i915_private *dev_priv, > * case we can set the different FPx0/1 to dynamically select > * between low and high frequency. > */ > - if (scan->hdisplay == fixed_mode->hdisplay && > - scan->hsync_start == fixed_mode->hsync_start && > - scan->hsync_end == fixed_mode->hsync_end && > - scan->htotal == fixed_mode->htotal && > - scan->vdisplay == fixed_mode->vdisplay && > - scan->vsync_start == fixed_mode->vsync_start && > - scan->vsync_end == fixed_mode->vsync_end && > - scan->vtotal == fixed_mode->vtotal) { > - if (scan->clock < temp_downclock) { > - /* > - * The downclock is already found. But we > - * expect to find the lower downclock. > - */ > - temp_downclock = scan->clock; > - tmp_mode = scan; > - } > + if (is_downclock_mode(scan, fixed_mode) && > + scan->clock < best_clock) { > + /* > + * The downclock is already found. But we > + * expect to find the lower downclock. > + */ > + best_clock = scan->clock; > + best_mode = scan; > } > } > > - if (temp_downclock < fixed_mode->clock) > - return drm_mode_duplicate(&dev_priv->drm, tmp_mode); > - else > - return NULL; > + if (best_mode) > + return drm_mode_duplicate(&dev_priv->drm, best_mode); > + > + return NULL; > } Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, Mar 21, 2019 at 05:23:30PM +0000, Chris Wilson wrote: > Quoting Ville Syrjala (2019-03-21 13:24:45) > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Utilize drm_mode_match() instead of hand rolling it when > > looking for the DRRS downclock mode. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_panel.c | 49 +++++++++++++++--------------- > > 1 file changed, 24 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > index cf111eba1a93..4727e74f7437 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > > drm_mode_set_crtcinfo(adjusted_mode, 0); > > } > > > > +static bool is_downclock_mode(const struct drm_display_mode *downclock_mode, > > + const struct drm_display_mode *fixed_mode) > > +{ > > + return drm_mode_match(downclock_mode, fixed_mode, > > + DRM_MODE_MATCH_TIMINGS | > > Adds m1->vscan == m2->vscan > > Makes sense. > > > + DRM_MODE_MATCH_FLAGS | > > + DRM_MODE_MATCH_3D_FLAGS) && > > Together adds m1->flags == m2->flags > > And also makes sense. Somehow I didn't even notice it wasn't checking the flags. But yeah, seems appropriate to make it check this.
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index cf111eba1a93..4727e74f7437 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, drm_mode_set_crtcinfo(adjusted_mode, 0); } +static bool is_downclock_mode(const struct drm_display_mode *downclock_mode, + const struct drm_display_mode *fixed_mode) +{ + return drm_mode_match(downclock_mode, fixed_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS) && + downclock_mode->clock < fixed_mode->clock; +} + /** * intel_find_panel_downclock - find the reduced downclock for LVDS in EDID * @dev_priv: i915 device instance @@ -60,11 +70,8 @@ intel_find_panel_downclock(struct drm_i915_private *dev_priv, struct drm_display_mode *fixed_mode, struct drm_connector *connector) { - struct drm_display_mode *scan, *tmp_mode; - int temp_downclock; - - temp_downclock = fixed_mode->clock; - tmp_mode = NULL; + const struct drm_display_mode *scan, *best_mode = NULL; + int best_clock = fixed_mode->clock; list_for_each_entry(scan, &connector->probed_modes, head) { /* @@ -74,29 +81,21 @@ intel_find_panel_downclock(struct drm_i915_private *dev_priv, * case we can set the different FPx0/1 to dynamically select * between low and high frequency. */ - if (scan->hdisplay == fixed_mode->hdisplay && - scan->hsync_start == fixed_mode->hsync_start && - scan->hsync_end == fixed_mode->hsync_end && - scan->htotal == fixed_mode->htotal && - scan->vdisplay == fixed_mode->vdisplay && - scan->vsync_start == fixed_mode->vsync_start && - scan->vsync_end == fixed_mode->vsync_end && - scan->vtotal == fixed_mode->vtotal) { - if (scan->clock < temp_downclock) { - /* - * The downclock is already found. But we - * expect to find the lower downclock. - */ - temp_downclock = scan->clock; - tmp_mode = scan; - } + if (is_downclock_mode(scan, fixed_mode) && + scan->clock < best_clock) { + /* + * The downclock is already found. But we + * expect to find the lower downclock. + */ + best_clock = scan->clock; + best_mode = scan; } } - if (temp_downclock < fixed_mode->clock) - return drm_mode_duplicate(&dev_priv->drm, tmp_mode); - else - return NULL; + if (best_mode) + return drm_mode_duplicate(&dev_priv->drm, best_mode); + + return NULL; } struct drm_display_mode *