From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm: Add rotation_property to mode_config and creating it Date: Tue, 29 Jul 2014 12:40:29 +0300 Message-ID: <20140729094029.GP27580@intel.com> References: <1405413629-4266-4-git-send-email-sonika.jindal@intel.com> <1405426417-18616-1-git-send-email-sonika.jindal@intel.com> <20140728152941.GL27580@intel.com> <20140728184722.GH4747@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 2DDC56E148 for ; Tue, 29 Jul 2014 02:40:38 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140728184722.GH4747@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: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote: > On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrj=E4l=E4 wrote: > > On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jindal@intel.com wrote: > > > From: Sonika Jindal > > > = > > > v2: Adding creation of rotation_property here. > > > = > > > Signed-off-by: Sonika Jindal > > > --- > > > drivers/gpu/drm/drm_crtc.c | 3 ++- > > > include/drm/drm_crtc.h | 1 + > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > = > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > > index 787631e..49c0747 100644 > > > --- a/drivers/gpu/drm/drm_crtc.c > > > +++ b/drivers/gpu/drm/drm_crtc.c > > > @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_prope= rties(struct drm_device *dev) > > > "type", drm_plane_type_enum_list, > > > ARRAY_SIZE(drm_plane_type_enum_list)); > > > dev->mode_config.plane_type_property =3D type; > > > - > > > + dev->mode_config.rotation_property =3D drm_mode_create_rotation_pro= perty(dev, > > > + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); > > = > > This might not make sense for other (!i915) hardware. And that's the > > reason why I had the driver create the property in the first place. > > = > > I think Daniel was thinking that we might want to expose all the bits > > regardless of what the hardware supports, but I don't like that idea. > > There are other properties (eg. alpha blending, csc stuff, etc.) that > > have the same problem of hardware supporting only a (potentially small) > > subset of the possible values. I'd rather we didn't make life harder > > for userspace when the kernel can already report that certain values > > will never work. > = > Well I'd like the property to be in some generic place so that fbcon can > unroate and that with the atomic stuff we can have rotation support in the > core structures. Which should help with argument checking. > = > But for rotation I don't think we should set it up in generic code, but in > i915. So the place where we keep it would be generic, the values would be > the sames, but the allowed set would differ depending upon platform or > driver. That would still fail if all the planes on the same device don't support the same rotation flags. Eg. on i915 we would have this problem if we exposed the old video overlay as a drm plane. And it wouldn't be the first piece of hardware where I've seen this kind of thing. -- = Ville Syrj=E4l=E4 Intel OTC