| Message ID | 1370879552-11290-1-git-send-email-jtzhou@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23:52 Mon 10 Jun , Jett.Zhou wrote: > From: Jing Xiang <jxiang@marvell.com> > > There is bug when switch dma of graphic layer and video layer, it > configured opposite bit, fix it. > > Signed-off-by: Jing Xiang <jxiang@marvell.com> > Signed-off-by: Jett.Zhou <jtzhou@marvell.com> do you need those 2 fixes for 3.10 or it can wait for 3.11? Best Regards, J. > --- > drivers/video/mmp/hw/mmp_ctrl.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c > index 3a8c2a3..8836053 100644 > --- a/drivers/video/mmp/hw/mmp_ctrl.c > +++ b/drivers/video/mmp/hw/mmp_ctrl.c > @@ -160,9 +160,9 @@ static void overlay_set_win(struct mmp_overlay *overlay, struct mmp_win *win) > > static void dmafetch_onoff(struct mmp_overlay *overlay, int on) > { > - u32 mask = overlay_is_vid(overlay) ? CFG_GRA_ENA_MASK : > - CFG_DMA_ENA_MASK; > - u32 enable = overlay_is_vid(overlay) ? CFG_GRA_ENA(1) : CFG_DMA_ENA(1); > + u32 mask = overlay_is_vid(overlay) ? CFG_DMA_ENA_MASK : > + CFG_GRA_ENA_MASK; > + u32 enable = overlay_is_vid(overlay) ? CFG_DMA_ENA(1) : CFG_GRA_ENA(1); > u32 tmp; > struct mmp_path *path = overlay->path; > > -- > 1.7.0.4 >
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote: > From: Jing Xiang <jxiang@marvell.com> > > There is bug when switch dma of graphic layer and video layer, it > configured opposite bit, fix it. So, overlays can be either graphics or video. overlay_is_vid() tells you which type. overlay_is_vid() makes its decision based on dmafetch_id which is undocumented and I don't understand it (and in another mail you said this is legacy configuration that is going to be removed). Ignoring my lack of understanding of the background: the patch here seems to make the code more correct. It will modify the path's underlying control register to enable graphics or video transfer depending on the overlay type. Turning a graphics overlay dmafetch ON will not stomp on the bits set by any video overlay dmafetch on the same path. Previously the logic was reversed. This codepath will not work correctly if there will ever be more than one overlay of the same type on the same path. For example, if we have two graphics overlays; 1. dmafetch_onoff(overlay1, on) - graphics transfer is now enabled in the hardware 2. dmafetch_onoff(overlay2, on) - graphics transfer was already enabled, no change 3. dmafetch_onoff(overlay2, off) - graphics transfer is now disabled in the hardware At this point overlay1 is still active in theory, but the driver has disabled graphics transfer at the hardware level. Daniel
2013/6/22 Daniel Drake <dsd@laptop.org>: > On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote: >> From: Jing Xiang <jxiang@marvell.com> >> >> There is bug when switch dma of graphic layer and video layer, it >> configured opposite bit, fix it. > > So, overlays can be either graphics or video. overlay_is_vid() tells > you which type. > overlay_is_vid() makes its decision based on dmafetch_id which is > undocumented and I don't understand it (and in another mail you said > this is legacy configuration that is going to be removed). > > Ignoring my lack of understanding of the background: the patch here > seems to make the code more correct. It will modify the path's > underlying control register to enable graphics or video transfer > depending on the overlay type. Turning a graphics overlay dmafetch ON > will not stomp on the bits set by any video overlay dmafetch on the > same path. Previously the logic was reversed. > > This codepath will not work correctly if there will ever be more than > one overlay of the same type on the same path. For example, if we have > two graphics overlays; > 1. dmafetch_onoff(overlay1, on) - graphics transfer is now enabled in > the hardware > 2. dmafetch_onoff(overlay2, on) - graphics transfer was already > enabled, no change > 3. dmafetch_onoff(overlay2, off) - graphics transfer is now disabled > in the hardware > > At this point overlay1 is still active in theory, but the driver has > disabled graphics transfer at the hardware level. > > Daniel Hi Daniel Now for our controller and usage, we have 2 over-lay of the path, they are graphic layer and video layer. dmafetch_id =0 if it is graphic layer, dmafetch_id =1 if it is video layer. The another mail mentioned is right, we plan to remove the dmafetch_id and detect overlay_is_vid by other method. But this patch did not include it yet, it will be included by another patch later on. Thanks -- ---------------------------------- Best Regards Jett Zhou
diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c index 3a8c2a3..8836053 100644 --- a/drivers/video/mmp/hw/mmp_ctrl.c +++ b/drivers/video/mmp/hw/mmp_ctrl.c @@ -160,9 +160,9 @@ static void overlay_set_win(struct mmp_overlay *overlay, struct mmp_win *win) static void dmafetch_onoff(struct mmp_overlay *overlay, int on) { - u32 mask = overlay_is_vid(overlay) ? CFG_GRA_ENA_MASK : - CFG_DMA_ENA_MASK; - u32 enable = overlay_is_vid(overlay) ? CFG_GRA_ENA(1) : CFG_DMA_ENA(1); + u32 mask = overlay_is_vid(overlay) ? CFG_DMA_ENA_MASK : + CFG_GRA_ENA_MASK; + u32 enable = overlay_is_vid(overlay) ? CFG_DMA_ENA(1) : CFG_GRA_ENA(1); u32 tmp; struct mmp_path *path = overlay->path;