>-----Original Message-----
>From: Ville Syrjala [mailto:[email protected]]
>Sent: Friday, January 11, 2019 10:38 PM
>To: [email protected]
>Cc: Shankar, Uma <[email protected]>; Roper, Matthew D
><[email protected]>
>Subject: [PATCH 11/13] drm/i915: Turn off pipe gamma when it's not needed.

Full stop can be dropped.

>
>From: Ville Syrjälä <[email protected]>
>
>The pipe internal precision is higher than what we currently program to the
>degamma/gamma LUTs. We can get a higher quality image by bypassing the LUTs
>when they're not needed. Let's do that.
>
>Each plane has its own control bit for this, so we have to update all active 
>planes.
>The way we've done this we don't actually have to run through the whole
>.check_plane() thing. And we actually do the .color_check() after 
>.check_plane()
>so we couldn't even do that without shuffling the code around.
>
>Additionally on pre-skl we have to update the primary plane regardless of
>whether it's active or not on account of the primayr plane gamma enable bit 
>also

Typo in primary.

Overall looks ok to me.
Reviewed-by: Uma Shankar <[email protected]>

>affecting the pipe bottom color.
>
>Signed-off-by: Ville Syrjälä <[email protected]>
>---
> drivers/gpu/drm/i915/intel_color.c | 55 ++++++++++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_color.c
>b/drivers/gpu/drm/i915/intel_color.c
>index 8d7ea902a34b..a8b7428a64bf 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -633,27 +633,78 @@ void intel_color_commit(const struct intel_crtc_state
>*crtc_state)
>       dev_priv->display.color_commit(crtc_state);
> }
>
>+static bool need_plane_update(struct intel_plane *plane,
>+                            const struct intel_crtc_state *crtc_state) {
>+      struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>+
>+      /*
>+       * On pre-SKL the pipe gamma enable and pipe csc enable for
>+       * the pipe bottom color are configured via the primary plane.
>+       * We have to reconfigure that even if the plane is inactive.
>+       */
>+      return crtc_state->active_planes & BIT(plane->id) ||
>+              (INTEL_GEN(dev_priv) < 9 &&
>+               plane->id == PLANE_PRIMARY);
>+}
>+
>+static int
>+intel_color_add_affected_planes(struct intel_crtc_state
>+*new_crtc_state) {
>+      struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
>+      struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+      struct intel_atomic_state *state =
>+              to_intel_atomic_state(new_crtc_state->base.state);
>+      const struct intel_crtc_state *old_crtc_state =
>+              intel_atomic_get_old_crtc_state(state, crtc);
>+      struct intel_plane *plane;
>+
>+      if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable)
>+              return 0;
>+
>+      for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
>+              struct intel_plane_state *plane_state;
>+
>+              if (!need_plane_update(plane, new_crtc_state))
>+                      continue;
>+
>+              plane_state = intel_atomic_get_plane_state(state, plane);
>+              if (IS_ERR(plane_state))
>+                      return PTR_ERR(plane_state);
>+
>+              new_crtc_state->update_planes |= BIT(plane->id);
>+      }
>+
>+      return 0;
>+}
>+
> int intel_color_check(struct intel_crtc_state *crtc_state)  {
>       struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>       const struct drm_property_blob *gamma_lut = crtc_state-
>>base.gamma_lut;
>       const struct drm_property_blob *degamma_lut = crtc_state-
>>base.degamma_lut;
>       size_t gamma_length, degamma_length;
>+      int ret;
>
>       degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
>       gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>
>-      crtc_state->gamma_enable = true;
>+      crtc_state->gamma_enable = gamma_lut || degamma_lut;
>
>       if (INTEL_GEN(dev_priv) >= 9 ||
>           IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
>               crtc_state->csc_enable = true;
>
>+      ret = intel_color_add_affected_planes(crtc_state);
>+      if (ret)
>+              return ret;
>+
>       /*
>        * We also allow no degamma lut/ctm and a gamma lut at the legacy
>        * size (256 entries).
>        */
>-      if (crtc_state_is_legacy_gamma(crtc_state)) {
>+      if (!crtc_state->gamma_enable ||
>+          crtc_state_is_legacy_gamma(crtc_state)) {
>               crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
>               return 0;
>       }
>--
>2.19.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to