Re: [PATCH 03/14] staging: media: tegra-vde: Prepare for interlacing support

2018-08-30 Thread Dan Carpenter 
On Mon, Aug 13, 2018 at 04:50:16PM +0200, Thierry Reding wrote:
>  static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
> + unsigned int num_ref_pics,
>   struct video_frame *dpb_frames,
>   unsigned int ref_frames_nb,
>   unsigned int with_earlier_poc_nb)
> @@ -251,13 +260,17 @@ static void tegra_vde_setup_iram_tables(struct 
> tegra_vde *vde,
>   u32 value, aux_addr;
>   int with_later_poc_nb;
>   unsigned int i, k;
> + size_t size;
> +
> + size = num_ref_pics * 4 * 8;
> + memset(vde->iram, 0, size);

I can't get behind the magical size calculation...  :(

regards,
dan carpenter

Re: [PATCH 03/14] staging: media: tegra-vde: Prepare for interlacing support

2018-08-18 Thread Dmitry Osipenko 
On Monday, 13 August 2018 17:50:16 MSK Thierry Reding wrote:
> From: Thierry Reding 
> 
> The number of frames doubles when decoding interlaced content and the
> structures describing the frames double in size. Take that into account
> to prepare for interlacing support.
> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/staging/media/tegra-vde/tegra-vde.c | 73 -
>  1 file changed, 58 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
> b/drivers/staging/media/tegra-vde/tegra-vde.c index
> 3027b11b11ae..1a40f6dff7c8 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -61,7 +61,9 @@ struct video_frame {
>  };
> 
>  struct tegra_vde_soc {
> + unsigned int num_ref_pics;
>   bool supports_ref_pic_marking;
> + bool supports_interlacing;
>  };
> 
>  struct tegra_vde {
> @@ -205,8 +207,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde
> *vde, u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
>   u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
>   u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
> + u32 value = y_addr >> 8;

Let's name it value0 for consistency.

> 
> - VDE_WR(y_addr  >> 8, vde->frameid + 0x000 + frameid * 4);
> + if (vde->soc->supports_interlacing)
> + value |= BIT(31);
> +
> + VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
>   VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
>   VDE_WR(cr_addr >> 8, vde->frameid + 0x180 + frameid * 4);
>   VDE_WR(value1,   vde->frameid + 0x080 + frameid * 4);
> @@ -229,20 +235,23 @@ static void tegra_setup_frameidx(struct tegra_vde
> *vde, }
> 
>  static void tegra_vde_setup_iram_entry(struct tegra_vde *vde,
> +unsigned int num_ref_pics,
>  unsigned int table,
>  unsigned int row,
>  u32 value1, u32 value2)
>  {
> + unsigned int entries = num_ref_pics * 2;
>   u32 *iram_tables = vde->iram;
> 
>   dev_dbg(vde->miscdev.parent, "IRAM table %u: row %u: 0x%08X 0x%08X\n",
>   table, row, value1, value2);
> 
> - iram_tables[0x20 * table + row * 2] = value1;
> - iram_tables[0x20 * table + row * 2 + 1] = value2;
> + iram_tables[entries * table + row * 2] = value1;
> + iram_tables[entries * table + row * 2 + 1] = value2;
>  }
> 
>  static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
> + unsigned int num_ref_pics,
>   struct video_frame *dpb_frames,
>   unsigned int ref_frames_nb,
>   unsigned int with_earlier_poc_nb)
> @@ -251,13 +260,17 @@ static void tegra_vde_setup_iram_tables(struct
> tegra_vde *vde, u32 value, aux_addr;
>   int with_later_poc_nb;
>   unsigned int i, k;
> + size_t size;
> +
> + size = num_ref_pics * 4 * 8;
> + memset(vde->iram, 0, size);

Is this memset() really needed or it is just because you're feeling 
uncomfortable that something is kept uninitialized?

> 
>   dev_dbg(vde->miscdev.parent, "DPB: Frame 0: frame_num = %d\n",
>   dpb_frames[0].frame_num);
> 
>   dev_dbg(vde->miscdev.parent, "REF L0:\n");
> 
> - for (i = 0; i < 16; i++) {
> + for (i = 0; i < num_ref_pics; i++) {
>   if (i < ref_frames_nb) {
>   frame = _frames[i + 1];
> 
> @@ -277,10 +290,14 @@ static void tegra_vde_setup_iram_tables(struct
> tegra_vde *vde, value = 0;
>   }
> 
> - tegra_vde_setup_iram_entry(vde, 0, i, value, aux_addr);
> - tegra_vde_setup_iram_entry(vde, 1, i, value, aux_addr);
> - tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
> - tegra_vde_setup_iram_entry(vde, 3, i, value, aux_addr);
> + tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
> +aux_addr);
> + tegra_vde_setup_iram_entry(vde, num_ref_pics, 1, i, value,
> +aux_addr);
> + tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
> +aux_addr);
> + tegra_vde_setup_iram_entry(vde, num_ref_pics, 3, i, value,
> +aux_addr);
>   }
> 
>   if (!(dpb_frames[0].flags & FLAG_B_FRAME))
> @@ -309,7 +326,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
> *vde, "\tFrame %d: frame_num = %d\n",
>   k + 1, frame->frame_num);
> 
> - tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
> + tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
> +aux_addr);
>   }
> 
>   for

[PATCH 03/14] staging: media: tegra-vde: Prepare for interlacing support

2018-08-13 Thread Thierry Reding 
From: Thierry Reding 

The number of frames doubles when decoding interlaced content and the
structures describing the frames double in size. Take that into account
to prepare for interlacing support.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 73 -
 1 file changed, 58 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 3027b11b11ae..1a40f6dff7c8 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -61,7 +61,9 @@ struct video_frame {
 };
 
 struct tegra_vde_soc {
+   unsigned int num_ref_pics;
bool supports_ref_pic_marking;
+   bool supports_interlacing;
 };
 
 struct tegra_vde {
@@ -205,8 +207,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
+   u32 value = y_addr >> 8;
 
-   VDE_WR(y_addr  >> 8, vde->frameid + 0x000 + frameid * 4);
+   if (vde->soc->supports_interlacing)
+   value |= BIT(31);
+
+   VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
VDE_WR(cr_addr >> 8, vde->frameid + 0x180 + frameid * 4);
VDE_WR(value1,   vde->frameid + 0x080 + frameid * 4);
@@ -229,20 +235,23 @@ static void tegra_setup_frameidx(struct tegra_vde *vde,
 }
 
 static void tegra_vde_setup_iram_entry(struct tegra_vde *vde,
+  unsigned int num_ref_pics,
   unsigned int table,
   unsigned int row,
   u32 value1, u32 value2)
 {
+   unsigned int entries = num_ref_pics * 2;
u32 *iram_tables = vde->iram;
 
dev_dbg(vde->miscdev.parent, "IRAM table %u: row %u: 0x%08X 0x%08X\n",
table, row, value1, value2);
 
-   iram_tables[0x20 * table + row * 2] = value1;
-   iram_tables[0x20 * table + row * 2 + 1] = value2;
+   iram_tables[entries * table + row * 2] = value1;
+   iram_tables[entries * table + row * 2 + 1] = value2;
 }
 
 static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
+   unsigned int num_ref_pics,
struct video_frame *dpb_frames,
unsigned int ref_frames_nb,
unsigned int with_earlier_poc_nb)
@@ -251,13 +260,17 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
u32 value, aux_addr;
int with_later_poc_nb;
unsigned int i, k;
+   size_t size;
+
+   size = num_ref_pics * 4 * 8;
+   memset(vde->iram, 0, size);
 
dev_dbg(vde->miscdev.parent, "DPB: Frame 0: frame_num = %d\n",
dpb_frames[0].frame_num);
 
dev_dbg(vde->miscdev.parent, "REF L0:\n");
 
-   for (i = 0; i < 16; i++) {
+   for (i = 0; i < num_ref_pics; i++) {
if (i < ref_frames_nb) {
frame = _frames[i + 1];
 
@@ -277,10 +290,14 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
value = 0;
}
 
-   tegra_vde_setup_iram_entry(vde, 0, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 1, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 3, i, value, aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 1, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 3, i, value,
+  aux_addr);
}
 
if (!(dpb_frames[0].flags & FLAG_B_FRAME))
@@ -309,7 +326,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
 
-   tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+  aux_addr);
}
 
for (k = 0; i < ref_frames_nb; i++, k++) {
@@ -326,7 +344,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
 
-