[PATCH 1/4] drm/omap: Fix missing includes

[PATCH 1/4] drm/omap: Fix missing includes

[Tomi Valkeinen]

With certain kernel config options many omapdrm files fail to compile
due to missing include of linux/gpio/consumer.h and linux/of.h.

This patch adds those includes.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/gpu/drm/omapdrm/displays/encoder-opa362.c           | 1 +
 drivers/gpu/drm/omapdrm/displays/panel-dpi.c                | 1 +
 drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 1 +
 drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c  | 1 +
 drivers/gpu/drm/omapdrm/dss/hdmi4.c                         | 1 +
 drivers/gpu/drm/omapdrm/dss/hdmi5.c                         | 1 +
 6 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
index 8c246c213e06..9f06a87e80c4 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index e780fd4f8b46..ddf4f3be9a3d 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
index 458f77bc473d..ac680e1de603 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
@@ -15,6 +15,7 @@
 #include <linux/spi/spi.h>
 #include <linux/mutex.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
index 529a017602e4..1de5146c83c0 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
@@ -11,6 +11,7 @@
 
 #include <linux/delay.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index f892ae157ff3..eef260ff420d 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -33,6 +33,7 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
+#include <linux/of.h>
 #include <video/omapdss.h>
 #include <sound/omap-hdmi-audio.h>
 
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index a43f7b10e113..417dff5eb7cb 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -38,6 +38,7 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
+#include <linux/of.h>
 #include <video/omapdss.h>
 #include <sound/omap-hdmi-audio.h>
 
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/omap: remove unnecessary pitch round-up

[Tomi Valkeinen]

omapdrm checks if the pixel stride is divisible by 8. This is meant to
ensure that the byte stride is 32, which is required by SGX.

The check is not correct, as it checks for pixels, not bytes, and thus
needlessly increases the stride for, e.g., NV12.

Also, SGX driver is not supported in the mainline, and the TI's SGX
driver nowadays does the memory allocation itself and doesn't rely on
omapdrm to figure out the correct pitch.

So we can just remove the whole roundup.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 0fbe17d0ec6f..bdce6ec64a07 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -244,10 +244,7 @@ static inline int align_pitch(int pitch, int width, int bpp)
 	int bytespp = (bpp + 7) / 8;
 	/* in case someone tries to feed us a completely bogus stride: */
 	pitch = max(pitch, width * bytespp);
-	/* PVR needs alignment to 8 pixels.. right now that is the most
-	 * restrictive stride requirement..
-	 */
-	return roundup(pitch, 8 * bytespp);
+	return pitch;
 }
 
 /* map crtc to vblank mask */
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/omap: remove align_pitch()

[Tomi Valkeinen]

The previous commit removed aligning the pitch to SGX's pitch
requirement from align_pitch(). What's left is effectively a function
that returns width * bytespp.

To clean up the driver, we can remove the function and have the
calculation inline in the two places which call align_pitch().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h   | 8 --------
 drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++---
 drivers/gpu/drm/omapdrm/omap_gem.c   | 3 ++-
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index bdce6ec64a07..c23829f0f1c2 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -239,14 +239,6 @@ struct dma_buf *omap_gem_prime_export(struct drm_device *dev,
 struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev,
 		struct dma_buf *buffer);
 
-static inline int align_pitch(int pitch, int width, int bpp)
-{
-	int bytespp = (bpp + 7) / 8;
-	/* in case someone tries to feed us a completely bogus stride: */
-	pitch = max(pitch, width * bytespp);
-	return pitch;
-}
-
 /* map crtc to vblank mask */
 uint32_t pipe2vbl(struct drm_crtc *crtc);
 struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 3cb16f0cf381..cb6af7757720 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -125,9 +125,8 @@ static int omap_fbdev_create(struct drm_fb_helper *helper,
 	mode_cmd.width = sizes->surface_width;
 	mode_cmd.height = sizes->surface_height;
 
-	mode_cmd.pitches[0] = align_pitch(
-			mode_cmd.width * ((sizes->surface_bpp + 7) / 8),
-			mode_cmd.width, sizes->surface_bpp);
+	mode_cmd.pitches[0] = mode_cmd.width *
+		DIV_ROUND_UP(sizes->surface_bpp, 8);
 
 	fbdev->ywrap_enabled = priv->has_dmm && ywrap_enabled;
 	if (fbdev->ywrap_enabled) {
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 907154f5b67c..7ea1e00d8ca6 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -660,7 +660,8 @@ int omap_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 {
 	union omap_gem_size gsize;
 
-	args->pitch = align_pitch(0, args->width, args->bpp);
+	args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
+
 	args->size = PAGE_ALIGN(args->pitch * args->height);
 
 	gsize = (union omap_gem_size){
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/omap: fix OMAP4 hdmi_core_powerdown_disable()

[Tomi Valkeinen]

hdmi_core_powerdown_disable() is supposed to disable HDMI core's
power-down mode. However, the function sets the power-down bit to 0,
which means "enable power-down".

This hasn't caused any issues as the PD seems to affect only interrupts
from HDMI core, and none of those interrupts are used at the moment. CEC
functionality requires core interrupts, and the PD mode needs to be
fixed.

This patch fixes hdmi_core_powerdown_disable() to actually disable the
PD mode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
index fa72e735dad2..ef3afe99e487 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
@@ -211,7 +211,7 @@ static void hdmi_core_init(struct hdmi_core_video_config *video_cfg)
 static void hdmi_core_powerdown_disable(struct hdmi_core_data *core)
 {
 	DSSDBG("Enter hdmi_core_powerdown_disable\n");
-	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x0, 0, 0);
+	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x1, 0, 0);
 }
 
 static void hdmi_core_swreset_release(struct hdmi_core_data *core)
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/omap: remove unnecessary pitch round-up

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Monday 18 Apr 2016 18:42:12 Tomi Valkeinen wrote:
> omapdrm checks if the pixel stride is divisible by 8. This is meant to
> ensure that the byte stride is 32, which is required by SGX.
> 
> The check is not correct, as it checks for pixels, not bytes, and thus
> needlessly increases the stride for, e.g., NV12.
> 
> Also, SGX driver is not supported in the mainline, and the TI's SGX
> driver nowadays does the memory allocation itself and doesn't rely on
> omapdrm to figure out the correct pitch.
> 
> So we can just remove the whole roundup.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 0fbe17d0ec6f..bdce6ec64a07
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -244,10 +244,7 @@ static inline int align_pitch(int pitch, int width, int
> bpp) int bytespp = (bpp + 7) / 8;
>  	/* in case someone tries to feed us a completely bogus stride: */
>  	pitch = max(pitch, width * bytespp);
> -	/* PVR needs alignment to 8 pixels.. right now that is the most
> -	 * restrictive stride requirement..
> -	 */
> -	return roundup(pitch, 8 * bytespp);
> +	return pitch;
>  }
> 
>  /* map crtc to vblank mask */

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/omap: remove align_pitch()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Monday 18 Apr 2016 18:42:13 Tomi Valkeinen wrote:
> The previous commit removed aligning the pitch to SGX's pitch
> requirement from align_pitch(). What's left is effectively a function
> that returns width * bytespp.
> 
> To clean up the driver, we can remove the function and have the
> calculation inline in the two places which call align_pitch().
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I wonder, however, if the calculation is correct. Shouldn't the pitch be 
calculated as DIV_ROUND_UP(width * bpp, 8) instead of width * 
DIV_ROUND_UP(bpp, 8) ?

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h   | 8 --------
>  drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 ++---
>  drivers/gpu/drm/omapdrm/omap_gem.c   | 3 ++-
>  3 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index bdce6ec64a07..c23829f0f1c2
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -239,14 +239,6 @@ struct dma_buf *omap_gem_prime_export(struct drm_device
> *dev, struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev,
> struct dma_buf *buffer);
> 
> -static inline int align_pitch(int pitch, int width, int bpp)
> -{
> -	int bytespp = (bpp + 7) / 8;
> -	/* in case someone tries to feed us a completely bogus stride: */
> -	pitch = max(pitch, width * bytespp);
> -	return pitch;
> -}
> -
>  /* map crtc to vblank mask */
>  uint32_t pipe2vbl(struct drm_crtc *crtc);
>  struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder
> *encoder); diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> b/drivers/gpu/drm/omapdrm/omap_fbdev.c index 3cb16f0cf381..cb6af7757720
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -125,9 +125,8 @@ static int omap_fbdev_create(struct drm_fb_helper
> *helper, mode_cmd.width = sizes->surface_width;
>  	mode_cmd.height = sizes->surface_height;
> 
> -	mode_cmd.pitches[0] = align_pitch(
> -			mode_cmd.width * ((sizes->surface_bpp + 7) / 8),
> -			mode_cmd.width, sizes->surface_bpp);
> +	mode_cmd.pitches[0] = mode_cmd.width *
> +		DIV_ROUND_UP(sizes->surface_bpp, 8);
> 
>  	fbdev->ywrap_enabled = priv->has_dmm && ywrap_enabled;
>  	if (fbdev->ywrap_enabled) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c index 907154f5b67c..7ea1e00d8ca6
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -660,7 +660,8 @@ int omap_gem_dumb_create(struct drm_file *file, struct
> drm_device *dev, {
>  	union omap_gem_size gsize;
> 
> -	args->pitch = align_pitch(0, args->width, args->bpp);
> +	args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> +
>  	args->size = PAGE_ALIGN(args->pitch * args->height);
> 
>  	gsize = (union omap_gem_size){

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/omap: Fix missing includes

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Monday 18 Apr 2016 18:42:11 Tomi Valkeinen wrote:
> With certain kernel config options many omapdrm files fail to compile
> due to missing include of linux/gpio/consumer.h and linux/of.h.
> 
> This patch adds those includes.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reported-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/displays/encoder-opa362.c           | 1 +
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c                | 1 +
>  drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 1 +
>  drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c  | 1 +
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c                         | 1 +
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c                         | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c index
> 8c246c213e06..9f06a87e80c4 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> @@ -15,6 +15,7 @@
>   */
> 
>  #include <linux/gpio.h>

This driver uses the gpiod API only, you can remove gpio.h and of_gpio.h.

> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c index
> e780fd4f8b46..ddf4f3be9a3d 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -10,6 +10,7 @@
>   */
> 
>  #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c index
> 458f77bc473d..ac680e1de603 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> @@ -15,6 +15,7 @@
>  #include <linux/spi/spi.h>
>  #include <linux/mutex.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> 
>  #include <video/omapdss.h>
>  #include <video/omap-panel-data.h>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c index
> 529a017602e4..1de5146c83c0 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> @@ -11,6 +11,7 @@
> 
>  #include <linux/delay.h>
>  #include <linux/gpio.h>

Same here.

With that fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index f892ae157ff3..eef260ff420d
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -33,6 +33,7 @@
>  #include <linux/gpio.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/component.h>
> +#include <linux/of.h>
>  #include <video/omapdss.h>
>  #include <sound/omap-hdmi-audio.h>
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index a43f7b10e113..417dff5eb7cb
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -38,6 +38,7 @@
>  #include <linux/gpio.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/component.h>
> +#include <linux/of.h>
>  #include <video/omapdss.h>
>  #include <sound/omap-hdmi-audio.h>

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/omap: Fix missing includes

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 3007 bytes --]


On 19/04/16 05:17, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Monday 18 Apr 2016 18:42:11 Tomi Valkeinen wrote:
>> With certain kernel config options many omapdrm files fail to compile
>> due to missing include of linux/gpio/consumer.h and linux/of.h.
>>
>> This patch adds those includes.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Reported-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/displays/encoder-opa362.c           | 1 +
>>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c                | 1 +
>>  drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 1 +
>>  drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c  | 1 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi4.c                         | 1 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi5.c                         | 1 +
>>  6 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
>> b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c index
>> 8c246c213e06..9f06a87e80c4 100644
>> --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
>> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
>> @@ -15,6 +15,7 @@
>>   */
>>
>>  #include <linux/gpio.h>
> 
> This driver uses the gpiod API only, you can remove gpio.h and of_gpio.h.

True. I don't want to mix fixes and cleanups, so I made a new patch for
that:

commit cdde6dcbf9fa0c5eadb42aac956a8ec3676aafb0 (HEAD -> 4.7/omapdrm)
Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date:   Tue Apr 19 08:33:25 2016 +0300

    drm/omap: remove unneeded gpio includes

    encoder-opa362.c and panel-sharp-ls037v7dw01.c do not use the legacy
    GPIO API, so we can remove the including of gpio.h and of_gpio.h.

    Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
index 9f06a87e80c4..e9b62a1bb5c5 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
@@ -14,12 +14,10 @@
  * the Free Software Foundation.
  */

-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/of_gpio.h>

 #include <video/omapdss.h>

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
index 1de5146c83c0..bbf529f15971 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
@@ -10,11 +10,9 @@
  */

 #include <linux/delay.h>
-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/regulator/consumer.h>


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/omap: remove align_pitch()

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 3321 bytes --]

On 19/04/16 05:12, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Monday 18 Apr 2016 18:42:13 Tomi Valkeinen wrote:
>> The previous commit removed aligning the pitch to SGX's pitch
>> requirement from align_pitch(). What's left is effectively a function
>> that returns width * bytespp.
>>
>> To clean up the driver, we can remove the function and have the
>> calculation inline in the two places which call align_pitch().
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I wonder, however, if the calculation is correct. Shouldn't the pitch be 
> calculated as DIV_ROUND_UP(width * bpp, 8) instead of width * 
> DIV_ROUND_UP(bpp, 8) ?

Well, in practice it doesn't matter. The bpps we use, 32/24/16 are all
divisible by 8. So I don't really even know why the div round up is there in
the first place.

There's the 12 bit RGB, in a 16 bit container, which in theory works but I
don't think we have ever used it or really supported it. If 'bpp' is 12 in this
case, width * DIV_ROUND_UP(bpp, 8) gives the correct pitch. But then, I guess
'bpp' should be 16 in this case, as the omap_gem.c part is about allocating
memory, without knowing the format.

Then there are the 1/2/4/8 bpp CLUT modes, which we have never supported (and
they are no longer supported by the HW in recent HW). For those,
DIV_ROUND_UP(width * bpp, 8) would be correct.

So... I think you are right. Here:

From c435ebfc5953d4983c5772ec4fb7c5b0d01ecb9f Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Tue, 19 Apr 2016 09:06:32 +0300
Subject: [PATCH] drm/omap: fix pitch round-up

At the moment we calculate the buffer's pitch with:

  pitch = width * DIV_ROUND_UP(bpp, 8)

For CLUT modes with bpp of 1/2/4/8 this gives wrong result, and the
correct pitch is:

  pitch = DIV_ROUND_UP(width * bpp, 8)

In practice this doesn't change anything, as we don't support CLUT
modes, but it's better to have the pitch calculation correct.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index cb6af7757720..42831b8c46b9 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -125,8 +125,8 @@ static int omap_fbdev_create(struct drm_fb_helper *helper,
 	mode_cmd.width = sizes->surface_width;
 	mode_cmd.height = sizes->surface_height;
 
-	mode_cmd.pitches[0] = mode_cmd.width *
-		DIV_ROUND_UP(sizes->surface_bpp, 8);
+	mode_cmd.pitches[0] =
+			DIV_ROUND_UP(mode_cmd.width * sizes->surface_bpp, 8);
 
 	fbdev->ywrap_enabled = priv->has_dmm && ywrap_enabled;
 	if (fbdev->ywrap_enabled) {
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 7ea1e00d8ca6..23af01eb1773 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -660,7 +660,7 @@ int omap_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 {
 	union omap_gem_size gsize;
 
-	args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
+	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
 
 	args->size = PAGE_ALIGN(args->pitch * args->height);
 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/omap: Fix missing includes

[Laurent Pinchart]

Hi Tomi,

On Tuesday 19 Apr 2016 08:36:01 Tomi Valkeinen wrote:
> On 19/04/16 05:17, Laurent Pinchart wrote:
> > On Monday 18 Apr 2016 18:42:11 Tomi Valkeinen wrote:
> >> With certain kernel config options many omapdrm files fail to compile
> >> due to missing include of linux/gpio/consumer.h and linux/of.h.
> >> 
> >> This patch adds those includes.
> >> 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> Reported-by: Dan Murphy <dmurphy@ti.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/omapdrm/displays/encoder-opa362.c           | 1 +
> >>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c                | 1 +
> >>  drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 1 +
> >>  drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c  | 1 +
> >>  drivers/gpu/drm/omapdrm/dss/hdmi4.c                         | 1 +
> >>  drivers/gpu/drm/omapdrm/dss/hdmi5.c                         | 1 +
> >>  6 files changed, 6 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> >> b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c index
> >> 8c246c213e06..9f06a87e80c4 100644
> >> --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> >> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> >> @@ -15,6 +15,7 @@
> >> 
> >>   */
> >>  
> >>  #include <linux/gpio.h>
> > 
> > This driver uses the gpiod API only, you can remove gpio.h and of_gpio.h.
> 
> True. I don't want to mix fixes and cleanups, so I made a new patch for
> that:

Given that you touch the two files in your "Fix missing includes" patch that 
seems a bit overkill to me, I would have squashed the two together, but it 
doesn't matter too much.

> commit cdde6dcbf9fa0c5eadb42aac956a8ec3676aafb0 (HEAD -> 4.7/omapdrm)
> Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date:   Tue Apr 19 08:33:25 2016 +0300
> 
>     drm/omap: remove unneeded gpio includes
> 
>     encoder-opa362.c and panel-sharp-ls037v7dw01.c do not use the legacy
>     GPIO API, so we can remove the including of gpio.h and of_gpio.h.
> 
>     Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> index 9f06a87e80c4..e9b62a1bb5c5 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> @@ -14,12 +14,10 @@
>   * the Free Software Foundation.
>   */
> 
> -#include <linux/gpio.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> -#include <linux/of_gpio.h>
> 
>  #include <video/omapdss.h>
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> index 1de5146c83c0..bbf529f15971 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> @@ -10,11 +10,9 @@
>   */
> 
>  #include <linux/delay.h>
> -#include <linux/gpio.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> -#include <linux/of_gpio.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/regulator/consumer.h>

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/omap: remove align_pitch()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 19 Apr 2016 09:15:23 Tomi Valkeinen wrote:
> On 19/04/16 05:12, Laurent Pinchart wrote:
> > On Monday 18 Apr 2016 18:42:13 Tomi Valkeinen wrote:
> >> The previous commit removed aligning the pitch to SGX's pitch
> >> requirement from align_pitch(). What's left is effectively a function
> >> that returns width * bytespp.
> >> 
> >> To clean up the driver, we can remove the function and have the
> >> calculation inline in the two places which call align_pitch().
> >> 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > I wonder, however, if the calculation is correct. Shouldn't the pitch be
> > calculated as DIV_ROUND_UP(width * bpp, 8) instead of width *
> > DIV_ROUND_UP(bpp, 8) ?
> 
> Well, in practice it doesn't matter. The bpps we use, 32/24/16 are all
> divisible by 8. So I don't really even know why the div round up is there in
> the first place.
> 
> There's the 12 bit RGB, in a 16 bit container, which in theory works but I
> don't think we have ever used it or really supported it. If 'bpp' is 12 in
> this case, width * DIV_ROUND_UP(bpp, 8) gives the correct pitch. But then,
> I guess 'bpp' should be 16 in this case, as the omap_gem.c part is about
> allocating memory, without knowing the format.
> 
> Then there are the 1/2/4/8 bpp CLUT modes, which we have never supported
> (and they are no longer supported by the HW in recent HW). For those,
> DIV_ROUND_UP(width * bpp, 8) would be correct.
> 
> So... I think you are right. Here:
> 
> From c435ebfc5953d4983c5772ec4fb7c5b0d01ecb9f Mon Sep 17 00:00:00 2001
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date: Tue, 19 Apr 2016 09:06:32 +0300
> Subject: [PATCH] drm/omap: fix pitch round-up
> 
> At the moment we calculate the buffer's pitch with:
> 
>   pitch = width * DIV_ROUND_UP(bpp, 8)
> 
> For CLUT modes with bpp of 1/2/4/8 this gives wrong result, and the
> correct pitch is:
> 
>   pitch = DIV_ROUND_UP(width * bpp, 8)
> 
> In practice this doesn't change anything, as we don't support CLUT
> modes, but it's better to have the pitch calculation correct.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> b/drivers/gpu/drm/omapdrm/omap_fbdev.c index cb6af7757720..42831b8c46b9
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -125,8 +125,8 @@ static int omap_fbdev_create(struct drm_fb_helper
> *helper, mode_cmd.width = sizes->surface_width;
>  	mode_cmd.height = sizes->surface_height;
> 
> -	mode_cmd.pitches[0] = mode_cmd.width *
> -		DIV_ROUND_UP(sizes->surface_bpp, 8);
> +	mode_cmd.pitches[0] =
> +			DIV_ROUND_UP(mode_cmd.width * sizes->surface_bpp, 8);
> 
>  	fbdev->ywrap_enabled = priv->has_dmm && ywrap_enabled;
>  	if (fbdev->ywrap_enabled) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c index 7ea1e00d8ca6..23af01eb1773
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -660,7 +660,7 @@ int omap_gem_dumb_create(struct drm_file *file, struct
> drm_device *dev, {
>  	union omap_gem_size gsize;
> 
> -	args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> +	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> 
>  	args->size = PAGE_ALIGN(args->pitch * args->height);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/omap: Fix missing includes

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 950 bytes --]

On 19/04/16 11:32, Laurent Pinchart wrote:

>>> This driver uses the gpiod API only, you can remove gpio.h and of_gpio.h.
>>
>> True. I don't want to mix fixes and cleanups, so I made a new patch for
>> that:
> 
> Given that you touch the two files in your "Fix missing includes" patch that 
> seems a bit overkill to me, I would have squashed the two together, but it 
> doesn't matter too much.

If the only thing the "Fix missing includes" patch would do is to touch
those two files and add the gpio/consumer.h, then... maybe. But it's
touching other files, and adding of.h too.

Maybe I'm being too pedantic, but I don't want to mix separate things
into the same patch. If in the description I write "This patch also does
xyz", I know I should split it up =).

I could have squashed the patch and just talked about "cleaning up the
includes". But that's not true, as the first patch is fixing compilation
issues.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel