Re: [PATCH 10/11] drm/i915: Add 180 degree primary plane rotation support

Tvrtko Ursulin
2014-06-27T11:12:32Z

On 06/27/2014 11:49 AM, Jindal, Sonika wrote:
>
>
> On 6/27/2014 4:04 PM, Tvrtko Ursulin wrote:
>> Hi,
>>
>> On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
>> [snip]
>>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>>> + struct drm_property *prop,
>>> + uint64_t val)
>>> +{
>>> + struct drm_device *dev = plane->dev;
>>> + struct drm_i915_private *dev_priv = dev->dev_private;
>>> + struct intel_plane *intel_plane = to_intel_plane(plane);
>>> + struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>>> + struct drm_crtc *crtc = &intel_crtc->base;
>>> + uint64_t old_val;
>>> + int ret = -ENOENT;
>>> +
>>> + if (prop == dev_priv->rotation_property) {
>>> + /* exactly one rotation angle please */
>>> + if (hweight32(val & 0xf) != 1)
>>> + return -EINVAL;
>>> +
>>> + old_val = intel_plane->rotation;
>>> + intel_plane->rotation = val;
>>> +
>>> + if (intel_crtc->active && intel_crtc->primary_enabled) {
>>> + intel_crtc_wait_for_pending_flips(crtc);
>>> +
>>> + /* FBC does not work on some platforms for rotated
>>> planes */
>>> + if (dev_priv->fbc.plane == intel_crtc->plane &&
>>> + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>>> + intel_plane->rotation != BIT(DRM_ROTATE_0))
>>> + intel_disable_fbc(dev);
>>> +
>>> + dev_priv->display.update_primary_plane(crtc,
>>> crtc->primary->fb, 0, 0);
>>> + } else {
>>> + DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation
>>> property is updated\n",
>>> + crtc->base.id);
>>> + ret = 0;
>>> + }
>>> + }
>>> +
>>> + return ret;
>>> +}
>>
>> It looks like this will incorrectly propagate -ENOENT if property on an
>> active plane is modified.
>>
>> Regards,
>>
>> Tvrtko
>>
>>
> Yes, this was corrected in the next patch set. Can you please refer to
> the latest patches where we moved the property to drm_plane instead of
> intel_plane:
> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047910.html

Alright, I missed that series since it is bit indented in the thread.
Does it replace only patch 10 from the original series? Or in other
words first nine should be applied first, then these three?

Tvrtko