Re: [PATCH 06/12] drm/i915: Pass plane to vlv_compute_drain_latency()

Jesse Barnes
2015-02-27T20:37:15Z

On 02/27/2015 10:09 AM, Ville Syrjälä wrote:
> On Fri, Feb 27, 2015 at 09:57:20AM -0800, Jesse Barnes wrote:
>> On 02/10/2015 05:28 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Now that we have drm_planes for the cursor and primary we can move the
>>> pixel_size handling into vlv_compute_drain_latency() and just pass the
>>> appropriate plane to it.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++-------------------------
>>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 5515d10..fffcf64 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -727,16 +727,26 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>>> }
>>>
>>> static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
>>> - int pixel_size)
>>> + struct drm_plane *plane)
>>> {
>>> struct drm_device *dev = crtc->dev;
>>> - int entries, prec_mult, drain_latency;
>>> - int clock = to_intel_crtc(crtc)->config->base.adjusted_mode.crtc_clock;
>>> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>> + int entries, prec_mult, drain_latency, pixel_size;
>>> + int clock = intel_crtc->config->base.adjusted_mode.crtc_clock;
>>> const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64;
>>>
>>> + /*
>>> + * FIXME the plane might have an fb
>>> + * but be invisible (eg. due to clipping)
>>> + */
>>> + if (!intel_crtc->active || !plane->fb)
>>> + return 0;
>>> +
>>> if (WARN(clock == 0, "Pixel clock is zero!\n"))
>>> return 0;
>>>
>>> + pixel_size = drm_format_plane_cpp(plane->fb->pixel_format, 0);
>>> +
>>> if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
>>> return 0;
>>>
>>> @@ -770,31 +780,11 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
>>> struct drm_device *dev = crtc->dev;
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>> - int pixel_size;
>>> enum pipe pipe = intel_crtc->pipe;
>>> struct vlv_wm_values wm = dev_priv->wm.vlv;
>>>
>>> - wm.ddl[pipe].primary = 0;
>>> - wm.ddl[pipe].cursor = 0;
>>> -
>>> - if (!intel_crtc_active(crtc)) {
>>> - vlv_write_wm_values(intel_crtc, &wm);
>>> - return;
>>> - }
>>> -
>>> - /* Primary plane Drain Latency */
>>> - pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */
>>> - wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, pixel_size);
>>> -
>>> - /* Cursor Drain Latency
>>> - * BPP is always 4 for cursor
>>> - */
>>> - pixel_size = 4;
>>> -
>>> - /* Program cursor DL only if it is enabled */
>>> - if (intel_crtc->cursor_base)
>>> - wm.ddl[pipe].cursor =
>>> - vlv_compute_drain_latency(crtc, pixel_size);
>>> + wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, crtc->primary);
>>> + wm.ddl[pipe].cursor = vlv_compute_drain_latency(crtc, crtc->cursor);
>>>
>>> vlv_write_wm_values(intel_crtc, &wm);
>>> }
>>> @@ -961,7 +951,7 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane,
>>>
>>> if (enabled)
>>> wm.ddl[pipe].sprite[sprite] =
>>> - vlv_compute_drain_latency(crtc, pixel_size);
>>> + vlv_compute_drain_latency(crtc, plane);
>>> else
>>> wm.ddl[pipe].sprite[sprite] = 0;
>>>
>>>
>>
>> Nice.
>
> Actually this is going to have issues now that atomic is partially in.
> We'd need to look at plane->state->fb now, but that steps on the same
> turf as Matt's latest patches.

Yeah there's some overlap, but mostly it looked like these would make
things a little easier by removing duplication and clearing things up
(not to mention the bug fixes).

Jesse