From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3 11/12] drm: Set property to return invalid for unsupported arguments for bitmask property Date: Thu, 6 Feb 2014 18:21:21 +0200 Message-ID: <20140206162121.GT3891@intel.com> References: <1391699093-12625-1-git-send-email-sagar.a.kamble@intel.com> <1391699093-12625-12-git-send-email-sagar.a.kamble@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id EC548FB706 for ; Thu, 6 Feb 2014 08:21:56 -0800 (PST) Content-Disposition: inline In-Reply-To: <1391699093-12625-12-git-send-email-sagar.a.kamble@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: sagar.a.kamble@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Feb 06, 2014 at 08:34:52PM +0530, sagar.a.kamble@intel.com wrote: > From: Sagar Kamble > = > DRM will not propagate the set_property call for bitmask drm > properties if they are not supported by underlying driver. > = > Signed-off-by: Sagar Kamble > Tested-by: Sagar Kamble > --- > drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > = > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 4f5e408..4c92741 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3420,6 +3420,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_devi= ce *dev, void *data, > struct drm_mode_object *arg_obj; > struct drm_mode_object *prop_obj; > struct drm_property *property; > + struct drm_property_enum *prop_enum; > + bool supported_val =3D false; > int ret =3D -EINVAL; > int i; > = > @@ -3451,6 +3453,22 @@ int drm_mode_obj_set_property_ioctl(struct drm_dev= ice *dev, void *data, > } > property =3D obj_to_property(prop_obj); > = > + if (property->flags | DRM_MODE_PROP_BITMASK) { > + if (!list_empty(&property->enum_blob_list)) { > + list_for_each_entry(prop_enum, > + &property->enum_blob_list, head) { > + if (BIT(prop_enum->value) =3D=3D arg->value) { > + supported_val =3D true; > + break; > + } > + } > + } > + if (!supported_val) { > + ret =3D -EINVAL; > + goto out; > + } > + } > + > if (!drm_property_change_is_valid(property, arg->value)) drm_property_change_is_valid() should take care of this, except I think my original patch to pass the supported_bits information is a bit buggy. It will allocate the values[] array based on all possible rotation bits, but then it will populate it sparsely based on the supported_bits. Apparently that needs to be changed to allocate only the required amount of space in values[]. I think that should make drm_property_change_is_valid() correctly detect the valid bits. Actually I think even my buggy patch should mostly work, it'll just always accept DRM_ROTATE_0 even if that wasnt't one of the supported_bits. That's because since values[] is zeroed initially. Oh, I see your patch does actually a bit more than that. It would reject any bitmask property change with multiple bits set. That's not what we want, as the driver is the only one that can decide which combination(s) of bits are in fact valid. -- = Ville Syrj=E4l=E4 Intel OTC