Thread (3 messages) 3 messages, 3 authors, 2009-05-18

Re: [Linux-fbdev-devel] [PATCH 01/11] video: Add support for the Avionic Design Xanthos framebuffer.

From: Thierry Reding <hidden>
Date: 2009-05-18 08:53:01

* Krzysztof Helt wrote:
Hi Thierry
[...]
quoted
quoted
+static int adxfb_ioctl(struct fb_info *info, unsigned int command,
+		unsigned long arg)
+{
+	struct adxfb_info *fb = to_adxfb_info(info);
+	void __user *argp = (void __user *)arg;
+	struct adxfb_scaler_mode mode;
+	struct adxfb_viewport viewport;
+	int err = 0;
+
+	switch (command) {
+	case ADXFB_IOCTL_SCALER_SET_MODE:
+		if (copy_from_user(&mode, argp, sizeof(mode)))
+			return -EFAULT;
+
+		err = adxfb_scaler_set_mode(info, &mode);
+		if (err < 0)
+			return err;
+
+		break;
+
+	case ADXFB_IOCTL_SCALER_GET_MODE:
+		err = adxfb_scaler_get_mode(info, &mode);
+		if (err < 0)
+			return err;
+
+		if (copy_to_user(argp, &mode, sizeof(mode)))
+			return -EFAULT;
+
+		break;
+
+	case ADXFB_IOCTL_OVERLAY_ENABLE:
+		err = adxfb_overlay_enable(info, arg);
+		if (err < 0)
+			return err;
+
+		break;
+
+	case ADXFB_IOCTL_OVERLAY_SET_VIEWPORT:
+		if (copy_from_user(&viewport, argp, sizeof(viewport)))
+			return -EFAULT;
+
+		err = adxfb_overlay_set_viewport(info, &viewport);
+		if (err < 0)
+			return err;
+
+		break;
+
+	default:
+		if (fb && fb->ioctl)
+			return fb->ioctl(info, command, arg);
+
+		break;
The fb->ioctl() is the adxfb_ioctl() here. It will cause infinite
recursion if called with unknown ioctl command.
Do not define the clause at all or return the -ENOTTY here.
It is not, actually. The fb->ioctl is supposed to allow board-specific
extensions and is usually NULL. It is not the same as adxfb_ioctl() but is
defined in the adxfb_info structure.

[...]
I have no comments to rest of the patch so I removed it.

I have a general comment that you should make your driver
conform to the fbdev API (check_var/set_par) for mode setting.
I know you have the scaler, so leave the adfxfb_ioctl to handle it.
If you define the set_par/check_var functions to handle size of
the overlay's input (without scaling) and color format one can
use standard FBIOPUT_VSCREENINFO and 
FBIOGET_VSCREENINFO ioctls to set and read your overlay.
If the overlay should be scaled use the custom ioctls to set the 
scaling factors only.
Currently, your driver looks like it wants to work around the
fbdev API. See the skeletonfb.c for guidance.
I was under the impression that FBIOPUT_SCREENINFO was used to set the
framebuffer resolution, which can be (even usually is) different from
that of the overlay. How can I distinguish between which of the video
pages (framebuffer or overlay) should be modified?
Regards,
Krzysztof
Thierry


-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help