Re: [PATCH] omapfb: Remove unused writeback code

2020-03-20 Thread Bartlomiej Zolnierkiewicz


On 3/13/20 1:24 PM, Tomi Valkeinen wrote:
> Remove unused writeback code. This code will never be used, as omapfb is
> being deprecated.
> 
> Signed-off-by: Tomi Valkeinen 

Patch queued for v5.7, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> ---
>  drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 114 ---
>  drivers/video/fbdev/omap2/omapfb/dss/dss.h   |  20 
>  2 files changed, 134 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c 
> b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> index ce37da85cc45..4a16798b2ecd 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -557,11 +557,6 @@ u32 dispc_mgr_get_sync_lost_irq(enum omap_channel 
> channel)
>  }
>  EXPORT_SYMBOL(dispc_mgr_get_sync_lost_irq);
>  
> -u32 dispc_wb_get_framedone_irq(void)
> -{
> - return DISPC_IRQ_FRAMEDONEWB;
> -}
> -
>  bool dispc_mgr_go_busy(enum omap_channel channel)
>  {
>   return mgr_fld_read(channel, DISPC_MGR_FLD_GO) == 1;
> @@ -579,30 +574,6 @@ void dispc_mgr_go(enum omap_channel channel)
>  }
>  EXPORT_SYMBOL(dispc_mgr_go);
>  
> -bool dispc_wb_go_busy(void)
> -{
> - return REG_GET(DISPC_CONTROL2, 6, 6) == 1;
> -}
> -
> -void dispc_wb_go(void)
> -{
> - enum omap_plane plane = OMAP_DSS_WB;
> - bool enable, go;
> -
> - enable = REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0) == 1;
> -
> - if (!enable)
> - return;
> -
> - go = REG_GET(DISPC_CONTROL2, 6, 6) == 1;
> - if (go) {
> - DSSERR("GO bit not down for WB\n");
> - return;
> - }
> -
> - REG_FLD_MOD(DISPC_CONTROL2, 1, 6, 6);
> -}
> -
>  static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 
> value)
>  {
>   dispc_write_reg(DISPC_OVL_FIR_COEF_H(plane, reg), value);
> @@ -1028,13 +999,6 @@ static enum omap_channel dispc_ovl_get_channel_out(enum 
> omap_plane plane)
>   }
>  }
>  
> -void dispc_wb_set_channel_in(enum dss_writeback_channel channel)
> -{
> - enum omap_plane plane = OMAP_DSS_WB;
> -
> - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), channel, 18, 16);
> -}
> -
>  static void dispc_ovl_set_burst_size(enum omap_plane plane,
>   enum omap_burst_size burst_size)
>  {
> @@ -2805,74 +2769,6 @@ int dispc_ovl_setup(enum omap_plane plane, const 
> struct omap_overlay_info *oi,
>  }
>  EXPORT_SYMBOL(dispc_ovl_setup);
>  
> -int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
> - bool mem_to_mem, const struct omap_video_timings *mgr_timings)
> -{
> - int r;
> - u32 l;
> - enum omap_plane plane = OMAP_DSS_WB;
> - const int pos_x = 0, pos_y = 0;
> - const u8 zorder = 0, global_alpha = 0;
> - const bool replication = false;
> - bool truncation;
> - int in_width = mgr_timings->x_res;
> - int in_height = mgr_timings->y_res;
> - enum omap_overlay_caps caps =
> - OMAP_DSS_OVL_CAP_SCALE | OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA;
> -
> - DSSDBG("dispc_wb_setup, pa %x, pa_uv %x, %d,%d -> %dx%d, cmode %x, "
> - "rot %d, mir %d\n", wi->paddr, wi->p_uv_addr, in_width,
> - in_height, wi->width, wi->height, wi->color_mode, wi->rotation,
> - wi->mirror);
> -
> - r = dispc_ovl_setup_common(plane, caps, wi->paddr, wi->p_uv_addr,
> - wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
> - wi->height, wi->color_mode, wi->rotation, wi->mirror, zorder,
> - wi->pre_mult_alpha, global_alpha, wi->rotation_type,
> - replication, mgr_timings, mem_to_mem);
> -
> - switch (wi->color_mode) {
> - case OMAP_DSS_COLOR_RGB16:
> - case OMAP_DSS_COLOR_RGB24P:
> - case OMAP_DSS_COLOR_ARGB16:
> - case OMAP_DSS_COLOR_RGBA16:
> - case OMAP_DSS_COLOR_RGB12U:
> - case OMAP_DSS_COLOR_ARGB16_1555:
> - case OMAP_DSS_COLOR_XRGB16_1555:
> - case OMAP_DSS_COLOR_RGBX16:
> - truncation = true;
> - break;
> - default:
> - truncation = false;
> - break;
> - }
> -
> - /* setup extra DISPC_WB_ATTRIBUTES */
> - l = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
> - l = FLD_MOD(l, truncation, 10, 10); /* TRUNCATIONENABLE */
> - l = FLD_MOD(l, mem_to_mem, 19, 19); /* WRITEBACKMODE */
> - if (mem_to_mem)
> - l = FLD_MOD(l, 1, 26, 24);  /* CAPTUREMODE */
> - else
> - l = FLD_MOD(l, 0, 26, 24);  /* CAPTUREMODE */
> - dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), l);
> -
> - if (mem_to_mem) {
> - /* WBDELAYCOUNT */
> - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES2(plane), 0, 7, 0);
> - } else {
> - int wbdelay;
> -
> - wbdelay = min(mgr_timings->vfp + mgr_timings->vsw +
> - mgr_timings->vbp, 255);
> -
> - /* WBDELAYCOUNT */
> - REG_FLD_MOD(DISPC_OVL_ATTRI

Re: [PATCH] omapfb: Remove unused writeback code

2020-03-13 Thread Tomi Valkeinen

On 13/03/2020 15:38, Laurent Pinchart wrote:

Hi Tomi,

On Fri, Mar 13, 2020 at 03:30:15PM +0200, Tomi Valkeinen wrote:

On 13/03/2020 15:22, Laurent Pinchart wrote:

On Fri, Mar 13, 2020 at 02:24:10PM +0200, Tomi Valkeinen wrote:

Remove unused writeback code. This code will never be used, as omapfb is
being deprecated.


I'm fine with that, but out of curiosity, is there any particular reason
to remove that code now instead of letting omapfb bitrot slowly ?


It conflicts with tidss's writeback code in TI kernel, when compiling tidss and 
omapfb into the
kernel. I thought this is the easiest way to resolve that, as all the removed 
code is dead code,
instead of trying to invent new names in tidss for a lot of functions.

Probably the functions in tidss still could use some renaming in the future, 
but I didn't want to be
forced to do that because of omapfb's dead code...


Could you do both ? :-) It's not good using too generic names in tidss.
You can just prefix the functions with tidss_. There's a risk of
conflict with omapdrm too if the names are too generic.


I will, but I didn't want to start doing that right now. tidss has just been merged to mainline, and 
tidss is still under active development, so doing such a change is an invitation to conflicts. I'd 
like things to settle down a bit first.


 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] omapfb: Remove unused writeback code

2020-03-13 Thread Tomi Valkeinen

On 13/03/2020 15:22, Laurent Pinchart wrote:

Hi Tomi,

Thank you for the patch.

On Fri, Mar 13, 2020 at 02:24:10PM +0200, Tomi Valkeinen wrote:

Remove unused writeback code. This code will never be used, as omapfb is
being deprecated.


I'm fine with that, but out of curiosity, is there any particular reason
to remove that code now instead of letting omapfb bitrot slowly ?


It conflicts with tidss's writeback code in TI kernel, when compiling tidss and omapfb into the 
kernel. I thought this is the easiest way to resolve that, as all the removed code is dead code, 
instead of trying to invent new names in tidss for a lot of functions.


Probably the functions in tidss still could use some renaming in the future, but I didn't want to be 
forced to do that because of omapfb's dead code...


So, not a super good reason, but on the other hand, removing dead code is 
always a good thing.

 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] omapfb: Remove unused writeback code

2020-03-13 Thread Laurent Pinchart
Hi Tomi,

On Fri, Mar 13, 2020 at 03:30:15PM +0200, Tomi Valkeinen wrote:
> On 13/03/2020 15:22, Laurent Pinchart wrote:
> > On Fri, Mar 13, 2020 at 02:24:10PM +0200, Tomi Valkeinen wrote:
> >> Remove unused writeback code. This code will never be used, as omapfb is
> >> being deprecated.
> > 
> > I'm fine with that, but out of curiosity, is there any particular reason
> > to remove that code now instead of letting omapfb bitrot slowly ?
> 
> It conflicts with tidss's writeback code in TI kernel, when compiling tidss 
> and omapfb into the 
> kernel. I thought this is the easiest way to resolve that, as all the removed 
> code is dead code, 
> instead of trying to invent new names in tidss for a lot of functions.
>
> Probably the functions in tidss still could use some renaming in the future, 
> but I didn't want to be 
> forced to do that because of omapfb's dead code...

Could you do both ? :-) It's not good using too generic names in tidss.
You can just prefix the functions with tidss_. There's a risk of
conflict with omapdrm too if the names are too generic.

> So, not a super good reason, but on the other hand, removing dead code is 
> always a good thing.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] omapfb: Remove unused writeback code

2020-03-13 Thread Laurent Pinchart
Hi Tomi,

Thank you for the patch.

On Fri, Mar 13, 2020 at 02:24:10PM +0200, Tomi Valkeinen wrote:
> Remove unused writeback code. This code will never be used, as omapfb is
> being deprecated.

I'm fine with that, but out of curiosity, is there any particular reason
to remove that code now instead of letting omapfb bitrot slowly ?

> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 114 ---
>  drivers/video/fbdev/omap2/omapfb/dss/dss.h   |  20 
>  2 files changed, 134 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c 
> b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> index ce37da85cc45..4a16798b2ecd 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -557,11 +557,6 @@ u32 dispc_mgr_get_sync_lost_irq(enum omap_channel 
> channel)
>  }
>  EXPORT_SYMBOL(dispc_mgr_get_sync_lost_irq);
>  
> -u32 dispc_wb_get_framedone_irq(void)
> -{
> - return DISPC_IRQ_FRAMEDONEWB;
> -}
> -
>  bool dispc_mgr_go_busy(enum omap_channel channel)
>  {
>   return mgr_fld_read(channel, DISPC_MGR_FLD_GO) == 1;
> @@ -579,30 +574,6 @@ void dispc_mgr_go(enum omap_channel channel)
>  }
>  EXPORT_SYMBOL(dispc_mgr_go);
>  
> -bool dispc_wb_go_busy(void)
> -{
> - return REG_GET(DISPC_CONTROL2, 6, 6) == 1;
> -}
> -
> -void dispc_wb_go(void)
> -{
> - enum omap_plane plane = OMAP_DSS_WB;
> - bool enable, go;
> -
> - enable = REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0) == 1;
> -
> - if (!enable)
> - return;
> -
> - go = REG_GET(DISPC_CONTROL2, 6, 6) == 1;
> - if (go) {
> - DSSERR("GO bit not down for WB\n");
> - return;
> - }
> -
> - REG_FLD_MOD(DISPC_CONTROL2, 1, 6, 6);
> -}
> -
>  static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 
> value)
>  {
>   dispc_write_reg(DISPC_OVL_FIR_COEF_H(plane, reg), value);
> @@ -1028,13 +999,6 @@ static enum omap_channel dispc_ovl_get_channel_out(enum 
> omap_plane plane)
>   }
>  }
>  
> -void dispc_wb_set_channel_in(enum dss_writeback_channel channel)
> -{
> - enum omap_plane plane = OMAP_DSS_WB;
> -
> - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), channel, 18, 16);
> -}
> -
>  static void dispc_ovl_set_burst_size(enum omap_plane plane,
>   enum omap_burst_size burst_size)
>  {
> @@ -2805,74 +2769,6 @@ int dispc_ovl_setup(enum omap_plane plane, const 
> struct omap_overlay_info *oi,
>  }
>  EXPORT_SYMBOL(dispc_ovl_setup);
>  
> -int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
> - bool mem_to_mem, const struct omap_video_timings *mgr_timings)
> -{
> - int r;
> - u32 l;
> - enum omap_plane plane = OMAP_DSS_WB;
> - const int pos_x = 0, pos_y = 0;
> - const u8 zorder = 0, global_alpha = 0;
> - const bool replication = false;
> - bool truncation;
> - int in_width = mgr_timings->x_res;
> - int in_height = mgr_timings->y_res;
> - enum omap_overlay_caps caps =
> - OMAP_DSS_OVL_CAP_SCALE | OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA;
> -
> - DSSDBG("dispc_wb_setup, pa %x, pa_uv %x, %d,%d -> %dx%d, cmode %x, "
> - "rot %d, mir %d\n", wi->paddr, wi->p_uv_addr, in_width,
> - in_height, wi->width, wi->height, wi->color_mode, wi->rotation,
> - wi->mirror);
> -
> - r = dispc_ovl_setup_common(plane, caps, wi->paddr, wi->p_uv_addr,
> - wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
> - wi->height, wi->color_mode, wi->rotation, wi->mirror, zorder,
> - wi->pre_mult_alpha, global_alpha, wi->rotation_type,
> - replication, mgr_timings, mem_to_mem);
> -
> - switch (wi->color_mode) {
> - case OMAP_DSS_COLOR_RGB16:
> - case OMAP_DSS_COLOR_RGB24P:
> - case OMAP_DSS_COLOR_ARGB16:
> - case OMAP_DSS_COLOR_RGBA16:
> - case OMAP_DSS_COLOR_RGB12U:
> - case OMAP_DSS_COLOR_ARGB16_1555:
> - case OMAP_DSS_COLOR_XRGB16_1555:
> - case OMAP_DSS_COLOR_RGBX16:
> - truncation = true;
> - break;
> - default:
> - truncation = false;
> - break;
> - }
> -
> - /* setup extra DISPC_WB_ATTRIBUTES */
> - l = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
> - l = FLD_MOD(l, truncation, 10, 10); /* TRUNCATIONENABLE */
> - l = FLD_MOD(l, mem_to_mem, 19, 19); /* WRITEBACKMODE */
> - if (mem_to_mem)
> - l = FLD_MOD(l, 1, 26, 24);  /* CAPTUREMODE */
> - else
> - l = FLD_MOD(l, 0, 26, 24);  /* CAPTUREMODE */
> - dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), l);
> -
> - if (mem_to_mem) {
> - /* WBDELAYCOUNT */
> - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES2(plane), 0, 7, 0);
> - } else {
> - int wbdelay;
> -
> - wbdelay = min(mgr_timings->vfp + mgr_timings->vsw +
> - mgr_timings->vbp, 255);
> -
> -   

[PATCH] omapfb: Remove unused writeback code

2020-03-13 Thread Tomi Valkeinen
Remove unused writeback code. This code will never be used, as omapfb is
being deprecated.

Signed-off-by: Tomi Valkeinen 
---
 drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 114 ---
 drivers/video/fbdev/omap2/omapfb/dss/dss.h   |  20 
 2 files changed, 134 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c 
b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
index ce37da85cc45..4a16798b2ecd 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
@@ -557,11 +557,6 @@ u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel)
 }
 EXPORT_SYMBOL(dispc_mgr_get_sync_lost_irq);
 
-u32 dispc_wb_get_framedone_irq(void)
-{
-   return DISPC_IRQ_FRAMEDONEWB;
-}
-
 bool dispc_mgr_go_busy(enum omap_channel channel)
 {
return mgr_fld_read(channel, DISPC_MGR_FLD_GO) == 1;
@@ -579,30 +574,6 @@ void dispc_mgr_go(enum omap_channel channel)
 }
 EXPORT_SYMBOL(dispc_mgr_go);
 
-bool dispc_wb_go_busy(void)
-{
-   return REG_GET(DISPC_CONTROL2, 6, 6) == 1;
-}
-
-void dispc_wb_go(void)
-{
-   enum omap_plane plane = OMAP_DSS_WB;
-   bool enable, go;
-
-   enable = REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0) == 1;
-
-   if (!enable)
-   return;
-
-   go = REG_GET(DISPC_CONTROL2, 6, 6) == 1;
-   if (go) {
-   DSSERR("GO bit not down for WB\n");
-   return;
-   }
-
-   REG_FLD_MOD(DISPC_CONTROL2, 1, 6, 6);
-}
-
 static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 value)
 {
dispc_write_reg(DISPC_OVL_FIR_COEF_H(plane, reg), value);
@@ -1028,13 +999,6 @@ static enum omap_channel dispc_ovl_get_channel_out(enum 
omap_plane plane)
}
 }
 
-void dispc_wb_set_channel_in(enum dss_writeback_channel channel)
-{
-   enum omap_plane plane = OMAP_DSS_WB;
-
-   REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), channel, 18, 16);
-}
-
 static void dispc_ovl_set_burst_size(enum omap_plane plane,
enum omap_burst_size burst_size)
 {
@@ -2805,74 +2769,6 @@ int dispc_ovl_setup(enum omap_plane plane, const struct 
omap_overlay_info *oi,
 }
 EXPORT_SYMBOL(dispc_ovl_setup);
 
-int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
-   bool mem_to_mem, const struct omap_video_timings *mgr_timings)
-{
-   int r;
-   u32 l;
-   enum omap_plane plane = OMAP_DSS_WB;
-   const int pos_x = 0, pos_y = 0;
-   const u8 zorder = 0, global_alpha = 0;
-   const bool replication = false;
-   bool truncation;
-   int in_width = mgr_timings->x_res;
-   int in_height = mgr_timings->y_res;
-   enum omap_overlay_caps caps =
-   OMAP_DSS_OVL_CAP_SCALE | OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA;
-
-   DSSDBG("dispc_wb_setup, pa %x, pa_uv %x, %d,%d -> %dx%d, cmode %x, "
-   "rot %d, mir %d\n", wi->paddr, wi->p_uv_addr, in_width,
-   in_height, wi->width, wi->height, wi->color_mode, wi->rotation,
-   wi->mirror);
-
-   r = dispc_ovl_setup_common(plane, caps, wi->paddr, wi->p_uv_addr,
-   wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
-   wi->height, wi->color_mode, wi->rotation, wi->mirror, zorder,
-   wi->pre_mult_alpha, global_alpha, wi->rotation_type,
-   replication, mgr_timings, mem_to_mem);
-
-   switch (wi->color_mode) {
-   case OMAP_DSS_COLOR_RGB16:
-   case OMAP_DSS_COLOR_RGB24P:
-   case OMAP_DSS_COLOR_ARGB16:
-   case OMAP_DSS_COLOR_RGBA16:
-   case OMAP_DSS_COLOR_RGB12U:
-   case OMAP_DSS_COLOR_ARGB16_1555:
-   case OMAP_DSS_COLOR_XRGB16_1555:
-   case OMAP_DSS_COLOR_RGBX16:
-   truncation = true;
-   break;
-   default:
-   truncation = false;
-   break;
-   }
-
-   /* setup extra DISPC_WB_ATTRIBUTES */
-   l = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
-   l = FLD_MOD(l, truncation, 10, 10); /* TRUNCATIONENABLE */
-   l = FLD_MOD(l, mem_to_mem, 19, 19); /* WRITEBACKMODE */
-   if (mem_to_mem)
-   l = FLD_MOD(l, 1, 26, 24);  /* CAPTUREMODE */
-   else
-   l = FLD_MOD(l, 0, 26, 24);  /* CAPTUREMODE */
-   dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), l);
-
-   if (mem_to_mem) {
-   /* WBDELAYCOUNT */
-   REG_FLD_MOD(DISPC_OVL_ATTRIBUTES2(plane), 0, 7, 0);
-   } else {
-   int wbdelay;
-
-   wbdelay = min(mgr_timings->vfp + mgr_timings->vsw +
-   mgr_timings->vbp, 255);
-
-   /* WBDELAYCOUNT */
-   REG_FLD_MOD(DISPC_OVL_ATTRIBUTES2(plane), wbdelay, 7, 0);
-   }
-
-   return r;
-}
-
 int dispc_ovl_enable(enum omap_plane plane, bool enable)
 {
DSSDBG("dispc_enable_plane %d, %d\n", plane, enable);
@@ -2903,16 +2799,6 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
 }
 EXPORT_SYMBOL(dispc_mgr_is_enabled);
 
-v