Re: [PATCH 1/3] drm/exynos: mixer: add 2x scaling to mixer_graph_buffer

2015-04-01 Thread Tobias Jakobi 

Hello Gustavo,

On 2015-03-27 14:11, Gustavo Padovan wrote:
If you can rebase this in on top of my series this would be really 
good.
like I said in my other mail, the series doesn't apply cleanly anymore, 
so I had to rebase your series.




Then it would just be:
static int mixer_setup_scale(const struct exynos_drm_plane *plane,
unsigned int *x_ratio, unsigned int *y_ratio)

Also that would automatically fix your other comment below [*].


 Use EPERM or ENOTSUPP. Or even true/false.
Will do!


 You need to fix style here

 if (mixer_setup_scale(win_data-src_width, win_data-src_height,
   win_data-crtc_width, win_data-crtc_height,
   x_ratio, y_ratio))
 return;
With [*] this would just be:
if (mixer_setup_scale(plane, x_ratio, y_ratio)) return;

What do you think?


Changes sounds good to me. Please go ahead and send a new patch. :)
I've integrated the changes and just sent it out together with two other 
small fixes.


Here's the important part:
https://patchwork.kernel.org/patch/6140451/


With best wishes,
Tobias

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drm/exynos: mixer: add 2x scaling to mixer_graph_buffer

2015-03-27 Thread Gustavo Padovan 
Hi Tobias,

2015-03-27 Tobias Jakobi tjak...@math.uni-bielefeld.de:

 Hello!
 
 
 Gustavo Padovan wrote:
  I would keep calling these two vars x_ratio and y_ratio. I don't see a 
  reason
  to change the name here.
 Right, I'm going to change this. Also I was thinking of basing the patch
 on your latest cleanup series (the 'drm/exynos: remove struct *_win_data
 abstraction on planes' one).

If you can rebase this in on top of my series this would be really good.

 
 Then it would just be:
 static int mixer_setup_scale(const struct exynos_drm_plane *plane,
   unsigned int *x_ratio, unsigned int *y_ratio)
 
 Also that would automatically fix your other comment below [*].
 
 
  Use EPERM or ENOTSUPP. Or even true/false.
 Will do!
 
 
  You need to fix style here
  
  if (mixer_setup_scale(win_data-src_width, win_data-src_height,
  
win_data-crtc_width, win_data-crtc_height,  
  
x_ratio, y_ratio))  
  
  return; 
 With [*] this would just be:
 if (mixer_setup_scale(plane, x_ratio, y_ratio)) return;
 
 What do you think?

Changes sounds good to me. Please go ahead and send a new patch. :)

Gustavo
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drm/exynos: mixer: add 2x scaling to mixer_graph_buffer

2015-03-26 Thread Tobias Jakobi 
Hello!


Gustavo Padovan wrote:
 I would keep calling these two vars x_ratio and y_ratio. I don't see a reason
 to change the name here.
Right, I'm going to change this. Also I was thinking of basing the patch
on your latest cleanup series (the 'drm/exynos: remove struct *_win_data
abstraction on planes' one).

Then it would just be:
static int mixer_setup_scale(const struct exynos_drm_plane *plane,
unsigned int *x_ratio, unsigned int *y_ratio)

Also that would automatically fix your other comment below [*].


 Use EPERM or ENOTSUPP. Or even true/false.
Will do!


 You need to fix style here
 
 if (mixer_setup_scale(win_data-src_width, win_data-src_height,  
   
   win_data-crtc_width, win_data-crtc_height,
   
   x_ratio, y_ratio))
   
 return; 
With [*] this would just be:
if (mixer_setup_scale(plane, x_ratio, y_ratio)) return;

What do you think?


 I think your patch is good after these things get fixes and we can go with it
 and drop mine. Then I'll just rebase the alpha channel fix patch on top of
 this one.
Might I suggest to extend the alpha channel patch in this way:
https://github.com/tobiasjakobi/linux-odroid/commit/e3aad184eda2cade4d59a874e459a8ff265ed75f

With this we get at least some pixelformat validation into the driver.


With best wishes,
Tobias


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drm/exynos: mixer: add 2x scaling to mixer_graph_buffer

2015-03-26 Thread Gustavo Padovan 
Hi Tobias,

2015-03-26 Tobias Jakobi tjak...@math.uni-bielefeld.de:

 While the VP (video processor) supports arbitrary scaling
 of its input, the mixer just supports a simple 2x (line
 doubling) scaling. Expose this functionality and exit
 early when an unsupported scaling configuration is
 encountered.
 
 This was tested with modetest's DRM plane test (from
 the libdrm test suite) on an Odroid-X2 (Exynos4412).
 
 Signed-off-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/exynos/exynos_mixer.c | 38 
 +--
  1 file changed, 32 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
 b/drivers/gpu/drm/exynos/exynos_mixer.c
 index df547b6..7c38d3b 100644
 --- a/drivers/gpu/drm/exynos/exynos_mixer.c
 +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
 @@ -521,12 +521,37 @@ static void mixer_layer_update(struct mixer_context 
 *ctx)
   mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
  }
  
 +static int mixer_setup_scale(unsigned int src_w, unsigned int src_h,
 + unsigned int dst_w, unsigned int dst_h,
 + unsigned int *scale_w, unsigned int *scale_h)

I would keep calling these two vars x_ratio and y_ratio. I don't see a reason
to change the name here.

 +{
 + if (dst_w != src_w) {
 + if (dst_w == 2 * src_w)
 + *scale_w = 1;
 + else
 + goto fail;
 + }
 +
 + if (dst_h != src_h) {
 + if (dst_h == 2 * src_h)
 + *scale_h = 1;
 + else
 + goto fail;
 + }
 +
 + return 0;
 +
 +fail:
 + DRM_DEBUG_KMS(only 2x width/height scaling of plane supported\n);
 + return -1;

Use EPERM or ENOTSUPP. Or even true/false.

 +}
 +
  static void mixer_graph_buffer(struct mixer_context *ctx, int win)
  {
   struct mixer_resources *res = ctx-mixer_res;
   unsigned long flags;
   struct hdmi_win_data *win_data;
 - unsigned int x_ratio, y_ratio;
 + unsigned int x_ratio = 0, y_ratio = 0;
   unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
   dma_addr_t dma_addr;
   unsigned int fmt;
 @@ -550,9 +575,10 @@ static void mixer_graph_buffer(struct mixer_context 
 *ctx, int win)
   fmt = ARGB;
   }
  
 - /* 2x scaling feature */
 - x_ratio = 0;
 - y_ratio = 0;
 + /* check if mixer supports scaling setup */
 + if (mixer_setup_scale(win_data-src_width, win_data-src_height,
 + win_data-crtc_width, win_data-crtc_height,
 + x_ratio, y_ratio)) return;

You need to fix style here

if (mixer_setup_scale(win_data-src_width, win_data-src_height,
  win_data-crtc_width, win_data-crtc_height,  
  x_ratio, y_ratio))  
return; 


I think your patch is good after these things get fixes and we can go with it
and drop mine. Then I'll just rebase the alpha channel fix patch on top of
this one.

Gustavo
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] drm/exynos: mixer: add 2x scaling to mixer_graph_buffer

2015-03-25 Thread Tobias Jakobi 
While the VP (video processor) supports arbitrary scaling
of its input, the mixer just supports a simple 2x (line
doubling) scaling. Expose this functionality and exit
early when an unsupported scaling configuration is
encountered.

This was tested with modetest's DRM plane test (from
the libdrm test suite) on an Odroid-X2 (Exynos4412).

Signed-off-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 38 +--
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index df547b6..7c38d3b 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -521,12 +521,37 @@ static void mixer_layer_update(struct mixer_context *ctx)
mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
 }
 
+static int mixer_setup_scale(unsigned int src_w, unsigned int src_h,
+   unsigned int dst_w, unsigned int dst_h,
+   unsigned int *scale_w, unsigned int *scale_h)
+{
+   if (dst_w != src_w) {
+   if (dst_w == 2 * src_w)
+   *scale_w = 1;
+   else
+   goto fail;
+   }
+
+   if (dst_h != src_h) {
+   if (dst_h == 2 * src_h)
+   *scale_h = 1;
+   else
+   goto fail;
+   }
+
+   return 0;
+
+fail:
+   DRM_DEBUG_KMS(only 2x width/height scaling of plane supported\n);
+   return -1;
+}
+
 static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 {
struct mixer_resources *res = ctx-mixer_res;
unsigned long flags;
struct hdmi_win_data *win_data;
-   unsigned int x_ratio, y_ratio;
+   unsigned int x_ratio = 0, y_ratio = 0;
unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
dma_addr_t dma_addr;
unsigned int fmt;
@@ -550,9 +575,10 @@ static void mixer_graph_buffer(struct mixer_context *ctx, 
int win)
fmt = ARGB;
}
 
-   /* 2x scaling feature */
-   x_ratio = 0;
-   y_ratio = 0;
+   /* check if mixer supports scaling setup */
+   if (mixer_setup_scale(win_data-src_width, win_data-src_height,
+   win_data-crtc_width, win_data-crtc_height,
+   x_ratio, y_ratio)) return;
 
dst_x_offset = win_data-crtc_x;
dst_y_offset = win_data-crtc_y;
@@ -588,8 +614,8 @@ static void mixer_graph_buffer(struct mixer_context *ctx, 
int win)
mixer_reg_write(res, MXR_RESOLUTION, val);
}
 
-   val  = MXR_GRP_WH_WIDTH(win_data-crtc_width);
-   val |= MXR_GRP_WH_HEIGHT(win_data-crtc_height);
+   val  = MXR_GRP_WH_WIDTH(win_data-src_width);
+   val |= MXR_GRP_WH_HEIGHT(win_data-src_height);
val |= MXR_GRP_WH_H_SCALE(x_ratio);
val |= MXR_GRP_WH_V_SCALE(y_ratio);
mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
-- 
2.0.5

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html