| Message ID | 20170222102909.18072-4-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 22, 2017 at 12:29:09PM +0200, Ander Conselvan de Oliveira wrote: > Geminilake scalers can do 7x7 filtering for all supported input sizes, > so it doesn't need the "high quality" mode programming, which was > actually removed from that platform. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_atomic.c | 14 ++++++++------ > drivers/gpu/drm/i915/intel_display.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 6 +++--- > 3 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index aa9160e..e65818e 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -121,7 +121,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > /** > * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests > - * @dev: DRM device > + * @dev_priv: i915 device This change makes the diff quite messy for what's essentially a addition of two lines of code. > * @crtc: intel crtc > * @crtc_state: incoming crtc_state to validate and setup scalers > * > @@ -136,9 +136,9 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > * 0 - scalers were setup succesfully > * error code - otherwise > */ > -int intel_atomic_setup_scalers(struct drm_device *dev, > - struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state) > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > + struct intel_crtc *intel_crtc, > + struct intel_crtc_state *crtc_state) > { > struct drm_plane *plane = NULL; > struct intel_plane *intel_plane; > @@ -199,7 +199,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > */ > if (!plane) { > struct drm_plane_state *state; > - plane = drm_plane_from_index(dev, i); > + plane = drm_plane_from_index(&dev_priv->drm, i); > state = drm_atomic_get_plane_state(drm_state, plane); > if (IS_ERR(state)) { > DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state\n", > @@ -247,7 +247,9 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > } > > /* set scaler mode */ > - if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > + if (IS_GEMINILAKE(dev_priv)) { > + scaler_state->scalers[*scaler_id].mode = 0; > + } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > /* > * when only 1 scaler is in use on either pipe A or B, > * scaler 0 operates in high quality (HQ) mode. > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 000fb31..65e76d1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10959,7 +10959,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > ret = skl_update_scaler_crtc(pipe_config); > > if (!ret) > - ret = intel_atomic_setup_scalers(dev, intel_crtc, > + ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, > pipe_config); > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 0edc499..d9d96d6 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1910,9 +1910,9 @@ intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, > return to_intel_plane_state(plane_state); > } > > -int intel_atomic_setup_scalers(struct drm_device *dev, > - struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state); > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > + struct intel_crtc *intel_crtc, > + struct intel_crtc_state *crtc_state); > > /* intel_atomic_plane.c */ > struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2017-02-22 at 16:41 +0200, Ville Syrjälä wrote: > On Wed, Feb 22, 2017 at 12:29:09PM +0200, Ander Conselvan de Oliveira wrote: > > Geminilake scalers can do 7x7 filtering for all supported input sizes, > > so it doesn't need the "high quality" mode programming, which was > > actually removed from that platform. > > > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > --- > > drivers/gpu/drm/i915/intel_atomic.c | 14 ++++++++------ > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > drivers/gpu/drm/i915/intel_drv.h | 6 +++--- > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > > index aa9160e..e65818e 100644 > > --- a/drivers/gpu/drm/i915/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/intel_atomic.c > > @@ -121,7 +121,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > > > /** > > * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests > > - * @dev: DRM device > > + * @dev_priv: i915 device > > This change makes the diff quite messy for what's essentially a > addition of two lines of code. There's no dev_priv in that function. While adding the dev_priv, it just seemed like pure laziness not to change the parameter. Didn't seem like a big deal since it has only one caller, but I'll split if you insist. Ander > > > * @crtc: intel crtc > > * @crtc_state: incoming crtc_state to validate and setup scalers > > * > > @@ -136,9 +136,9 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > * 0 - scalers were setup succesfully > > * error code - otherwise > > */ > > -int intel_atomic_setup_scalers(struct drm_device *dev, > > - struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state) > > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > > + struct intel_crtc *intel_crtc, > > + struct intel_crtc_state *crtc_state) > > { > > struct drm_plane *plane = NULL; > > struct intel_plane *intel_plane; > > @@ -199,7 +199,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > > */ > > if (!plane) { > > struct drm_plane_state *state; > > - plane = drm_plane_from_index(dev, i); > > + plane = drm_plane_from_index(&dev_priv->drm, i); > > state = drm_atomic_get_plane_state(drm_state, plane); > > if (IS_ERR(state)) { > > DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state\n", > > @@ -247,7 +247,9 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > > } > > > > /* set scaler mode */ > > - if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > > + if (IS_GEMINILAKE(dev_priv)) { > > + scaler_state->scalers[*scaler_id].mode = 0; > > + } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > > /* > > * when only 1 scaler is in use on either pipe A or B, > > * scaler 0 operates in high quality (HQ) mode. > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 000fb31..65e76d1 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -10959,7 +10959,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > ret = skl_update_scaler_crtc(pipe_config); > > > > if (!ret) > > - ret = intel_atomic_setup_scalers(dev, intel_crtc, > > + ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, > > pipe_config); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 0edc499..d9d96d6 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1910,9 +1910,9 @@ intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, > > return to_intel_plane_state(plane_state); > > } > > > > -int intel_atomic_setup_scalers(struct drm_device *dev, > > - struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state); > > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > > + struct intel_crtc *intel_crtc, > > + struct intel_crtc_state *crtc_state); > > > > /* intel_atomic_plane.c */ > > struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); > > -- > > 2.9.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On Wed, Feb 22, 2017 at 04:44:10PM +0200, Ander Conselvan De Oliveira wrote: > On Wed, 2017-02-22 at 16:41 +0200, Ville Syrjälä wrote: > > On Wed, Feb 22, 2017 at 12:29:09PM +0200, Ander Conselvan de Oliveira wrote: > > > Geminilake scalers can do 7x7 filtering for all supported input sizes, > > > so it doesn't need the "high quality" mode programming, which was > > > actually removed from that platform. > > > > > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_atomic.c | 14 ++++++++------ > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > drivers/gpu/drm/i915/intel_drv.h | 6 +++--- > > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > > > index aa9160e..e65818e 100644 > > > --- a/drivers/gpu/drm/i915/intel_atomic.c > > > +++ b/drivers/gpu/drm/i915/intel_atomic.c > > > @@ -121,7 +121,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > > > > > /** > > > * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests > > > - * @dev: DRM device > > > + * @dev_priv: i915 device > > > > This change makes the diff quite messy for what's essentially a > > addition of two lines of code. > > There's no dev_priv in that function. While adding the dev_priv, it just seemed > like pure laziness not to change the parameter. Didn't seem like a big deal > since it has only one caller, but I'll split if you insist. to_i915(dev) still works, so there's no real need for changing the parameter at this time. I think sneaking in cosmetic changes should only be done if it's really just a few lines, whereas in this case the cosmetic change makes up most of the patch leading to a very low signal to noise ratio. > > Ander > > > > > > * @crtc: intel crtc > > > * @crtc_state: incoming crtc_state to validate and setup scalers > > > * > > > @@ -136,9 +136,9 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > > * 0 - scalers were setup succesfully > > > * error code - otherwise > > > */ > > > -int intel_atomic_setup_scalers(struct drm_device *dev, > > > - struct intel_crtc *intel_crtc, > > > - struct intel_crtc_state *crtc_state) > > > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > > > + struct intel_crtc *intel_crtc, > > > + struct intel_crtc_state *crtc_state) > > > { > > > struct drm_plane *plane = NULL; > > > struct intel_plane *intel_plane; > > > @@ -199,7 +199,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > > > */ > > > if (!plane) { > > > struct drm_plane_state *state; > > > - plane = drm_plane_from_index(dev, i); > > > + plane = drm_plane_from_index(&dev_priv->drm, i); > > > state = drm_atomic_get_plane_state(drm_state, plane); > > > if (IS_ERR(state)) { > > > DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state\n", > > > @@ -247,7 +247,9 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > > > } > > > > > > /* set scaler mode */ > > > - if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > > > + if (IS_GEMINILAKE(dev_priv)) { > > > + scaler_state->scalers[*scaler_id].mode = 0; > > > + } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { > > > /* > > > * when only 1 scaler is in use on either pipe A or B, > > > * scaler 0 operates in high quality (HQ) mode. > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 000fb31..65e76d1 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -10959,7 +10959,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > > ret = skl_update_scaler_crtc(pipe_config); > > > > > > if (!ret) > > > - ret = intel_atomic_setup_scalers(dev, intel_crtc, > > > + ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, > > > pipe_config); > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 0edc499..d9d96d6 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1910,9 +1910,9 @@ intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, > > > return to_intel_plane_state(plane_state); > > > } > > > > > > -int intel_atomic_setup_scalers(struct drm_device *dev, > > > - struct intel_crtc *intel_crtc, > > > - struct intel_crtc_state *crtc_state); > > > +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > > > + struct intel_crtc *intel_crtc, > > > + struct intel_crtc_state *crtc_state); > > > > > > /* intel_atomic_plane.c */ > > > struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); > > > -- > > > 2.9.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index aa9160e..e65818e 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -121,7 +121,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, /** * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests - * @dev: DRM device + * @dev_priv: i915 device * @crtc: intel crtc * @crtc_state: incoming crtc_state to validate and setup scalers * @@ -136,9 +136,9 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, * 0 - scalers were setup succesfully * error code - otherwise */ -int intel_atomic_setup_scalers(struct drm_device *dev, - struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state) +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, + struct intel_crtc *intel_crtc, + struct intel_crtc_state *crtc_state) { struct drm_plane *plane = NULL; struct intel_plane *intel_plane; @@ -199,7 +199,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev, */ if (!plane) { struct drm_plane_state *state; - plane = drm_plane_from_index(dev, i); + plane = drm_plane_from_index(&dev_priv->drm, i); state = drm_atomic_get_plane_state(drm_state, plane); if (IS_ERR(state)) { DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state\n", @@ -247,7 +247,9 @@ int intel_atomic_setup_scalers(struct drm_device *dev, } /* set scaler mode */ - if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { + if (IS_GEMINILAKE(dev_priv)) { + scaler_state->scalers[*scaler_id].mode = 0; + } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) { /* * when only 1 scaler is in use on either pipe A or B, * scaler 0 operates in high quality (HQ) mode. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 000fb31..65e76d1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10959,7 +10959,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, ret = skl_update_scaler_crtc(pipe_config); if (!ret) - ret = intel_atomic_setup_scalers(dev, intel_crtc, + ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, pipe_config); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 0edc499..d9d96d6 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1910,9 +1910,9 @@ intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, return to_intel_plane_state(plane_state); } -int intel_atomic_setup_scalers(struct drm_device *dev, - struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state); +int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, + struct intel_crtc *intel_crtc, + struct intel_crtc_state *crtc_state); /* intel_atomic_plane.c */ struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
Geminilake scalers can do 7x7 filtering for all supported input sizes, so it doesn't need the "high quality" mode programming, which was actually removed from that platform. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_atomic.c | 14 ++++++++------ drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- 3 files changed, 12 insertions(+), 10 deletions(-)