From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jindal, Sonika" Subject: Re: [PATCH 10/11] drm/i915: Add 180 degree primary plane rotation support Date: Thu, 19 Jun 2014 13:22:25 +0530 Message-ID: <53A296B9.6080506@intel.com> References: <1403081847-4364-1-git-send-email-sonika.jindal@intel.com> <1403081847-4364-11-git-send-email-sonika.jindal@intel.com> <20140618170248.GF18833@strange.amr.corp.intel.com> <53A286AA.6030204@intel.com> <20140619070700.GO5821@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 16AA66E015 for ; Thu, 19 Jun 2014 00:52:36 -0700 (PDT) In-Reply-To: <20140619070700.GO5821@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: "Kamble, Sagar A" , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 6/19/2014 12:37 PM, Daniel Vetter wrote: > On Thu, Jun 19, 2014 at 12:13:54PM +0530, Jindal, Sonika wrote: >> On 6/18/2014 10:32 PM, Damien Lespiau wrote: >>> On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@intel.com wrote: >>>> From: Sonika Jindal > > >>>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c >>>> index 5e583a1..4c91fbc 100644 >>>> --- a/drivers/gpu/drm/i915/i915_dma.c >>>> +++ b/drivers/gpu/drm/i915/i915_dma.c >>>> @@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file) >>>> void i915_driver_lastclose(struct drm_device *dev) >>>> { >>>> struct drm_i915_private *dev_priv = dev->dev_private; >>>> + struct intel_crtc *crtc; >>>> + struct intel_plane *plane; >>>> >>>> /* On gen6+ we refuse to init without kms enabled, but then the drm core >>>> * goes right around and calls lastclose. Check for this and don't clean >>>> @@ -1946,6 +1948,21 @@ void i915_driver_lastclose(struct drm_device *dev) >>>> if (!dev_priv) >>>> return; >>>> >>>> + if (dev_priv->rotation_property) { >>>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { >>>> + to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0); >>>> + drm_object_property_set_value(&crtc->base.base, >>>> + dev_priv->rotation_property, >>>> + to_intel_plane(crtc->base.primary)->rotation); >>>> + } >>>> + list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) { >>>> + plane->rotation = BIT(DRM_ROTATE_0); >>>> + drm_object_property_set_value(&plane->base.base, >>>> + dev_priv->rotation_property, >>>> + plane->rotation); >>>> + } >>>> + } >>>> + >>> >>> The purpose of this seems to be restoring rotation to 0 and rely on the next >>> modeset to re-program the register. This code, is orthogonal to the pure >>> primary plane rotation enabling, spans through both sprite and primary planes >>> and may actually be a bit tricky to get right. >>> >>> -> This shouldn't be part of the same commit as the primary plane rotation. >>> Please span a new commit for it. >> Sure I will add another patch. >>> >>> Now, we also need something like this when switching VT, and probably for >>> kernel panic and debug handling as well, so lastclose() is not enough (and we >>> can probably do better). >>> >>> One idea would be for the core DRM code to know about this property, and make >>> sure we put the rotation back to 0 in restore_fbdev_mode(), we do something >>> similar for the for the sprite planes in there already. Another idea would be >>> to add a vfunc to execute driver specific code there in restore_fbdev_mode(). >>> >>> There is probably a better way to do it, I have to say I'm not super familiar >>> with this part of the driver. >>> >> I see that in omap driver too it is done in lastclose of the driver. Also, >> from driver fbdev_restore is only called during lastclose. >> Again I don't have more knowledge on this. >> Can we keep it here in this lastclose function to comply with omap driver? > > No, this really should be done in > drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the > restore_fbdev_mode function in there. Once that's done and once omap is > also using the generic rotation properties (I think it is already) we can > remove the rotation handling code from omap's last_close. Please also > throw a (compile-tested-only) patch on top for that so that Rob Clark can > pick it up. > -Daniel > Ok, I will add it. So should I add a function pointer say reset_properties in crtc, which will be called from restore_fbdev_mode?