From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 4/4] drm: Renesas SH Mobile DRM driver Date: Wed, 30 May 2012 17:16:46 +0300 Message-ID: <20120530141646.GT13065@intel.com> References: <1338381179-13290-1-git-send-email-laurent.pinchart@ideasonboard.com> <1338381179-13290-5-git-send-email-laurent.pinchart@ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 9F7C49E954 for ; Wed, 30 May 2012 07:16:49 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1338381179-13290-5-git-send-email-laurent.pinchart@ideasonboard.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, May 30, 2012 at 02:32:59PM +0200, Laurent Pinchart wrote: > +static struct drm_framebuffer * > +shmob_drm_fb_create(struct drm_device *dev, struct drm_file *file_priv, > + struct drm_mode_fb_cmd2 *mode_cmd) > +{ > + const struct shmob_drm_format_info *format; > + struct shmob_drm_framebuffer *sfb; > + struct drm_framebuffer *fb; > + struct drm_gem_object *obj; > + unsigned int i; > + int ret; > + > + format =3D shmob_drm_format_info(mode_cmd->pixel_format); > + if (format =3D=3D NULL) { > + dev_dbg(dev->dev, "unsupported pixel format %08x\n", > + mode_cmd->pixel_format); > + return ERR_PTR(-EINVAL); > + } > + > + sfb =3D kzalloc(sizeof(*fb), GFP_KERNEL); > + if (sfb =3D=3D NULL) > + return ERR_PTR(-ENOMEM); > + > + sfb->format =3D format; > + fb =3D &sfb->base; > + > + for (i =3D 0; i < (format->yuv ? 2 : 1); ++i) { > + obj =3D drm_gem_object_lookup(dev, file_priv, > + mode_cmd->handles[i]); > + if (obj =3D=3D NULL) { > + dev_dbg(dev->dev, "GEM object %u not found\n", > + mode_cmd->handles[i]); > + ret =3D -ENOENT; > + goto error; > + } > + sfb->sobj[i] =3D to_shmob_gem_object(obj); > + } offsets[] not checked, nor is it handled in the code that calculates the final offsets. Based on the rest of the code, it seems that the hardware assumes pitches[1] =3D=3D pitches[0]*chroma_cpp. You should reject other values here. Also it's not clear from the code if there are other stride limitations that would need checking. Also there are no checks to make sure the fb fits inside the gem object. At one point I tried to cook up a generic function to help with such checks. I probably need to revisit that issue and try to make something that'd work for tiled formats as well. -- = Ville Syrj=E4l=E4 Intel OTC