| Message ID | 1454162331-23555-1-git-send-email-parkch98@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Chanho, 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: > From: Chanho Park <chanho61.park@samsung.com> > > This patch adds a mic_bypass option to bypass the mic > from display out path. The mic(Mobile image compressor) compresses > RGB data from fimd and send the compressed data to the mipi dsi. > The bypass option can be founded from system register and the bit > of the option is 11. > > Cc: Inki Dae <inki.dae@samsung.com> > Cc: Joonyoung Shim <jy0922.shim@samsung.com> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com> > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > --- > .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt > index 27c3ce0..7f90c4a 100644 > --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt > +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt > @@ -45,6 +45,8 @@ Optional Properties: > Can be used in case timings cannot be provided otherwise > or to override timings provided by the panel. > - samsung,sysreg: handle to syscon used to control the system registers > +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. > + This option is only available since exynos5420. > - i80-if-timings: timing configuration for lcd i80 interface support. > - cs-setup: clock cycles for the active period of address signal is enabled > until chip select is enabled. > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 70194d0..4fb2952 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -94,6 +94,7 @@ struct fimd_driver_data { > unsigned int lcdblk_offset; > unsigned int lcdblk_vt_shift; > unsigned int lcdblk_bypass_shift; > + unsigned int lcdblk_mic_bypass_shift; > > unsigned int has_shadowcon:1; > unsigned int has_clksel:1; > @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { > .lcdblk_offset = 0x214, > .lcdblk_vt_shift = 24, > .lcdblk_bypass_shift = 15, > + .lcdblk_mic_bypass_shift = 11, > .has_shadowcon = 1, > .has_vidoutcon = 1, > .has_vtsel = 1, > @@ -162,6 +164,7 @@ struct fimd_context { > u32 i80ifcon; > bool i80_if; > bool suspended; > + bool mic_bypass; > int pipe; > wait_queue_head_t wait_vsync_queue; > atomic_t wait_vsync_event; > @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) > return; > } > > + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, > + driver_data->lcdblk_offset, > + 0x1 << driver_data->lcdblk_mic_bypass_shift, > + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { > + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); > + return; > + } It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, if (ctx->lcdblk_mic_bypass_shift) { if (ctx->sysreg && regmap_update_bits(ctx->sysreg, driver_data->lcdblk_offset, ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift, ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift)) { ... } } Thanks, Inki Dae > + > /* setup horizontal and vertical display size. */ > val = VIDTCON2_LINEVAL(mode->vdisplay - 1) | > VIDTCON2_HOZVAL(mode->hdisplay - 1) | > @@ -1014,6 +1025,9 @@ static int fimd_probe(struct platform_device *pdev) > if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) > ctx->vidcon1 |= VIDCON1_INV_VCLK; > > + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass")) > + ctx->mic_bypass = true; > + > i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings"); > if (i80_if_timings) { > u32 val; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Inki, On Thu, Feb 11, 2016 at 7:37 PM, Inki Dae <inki.dae@samsung.com> wrote: > Hi Chanho, > > 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: >> From: Chanho Park <chanho61.park@samsung.com> >> >> This patch adds a mic_bypass option to bypass the mic >> from display out path. The mic(Mobile image compressor) compresses >> RGB data from fimd and send the compressed data to the mipi dsi. >> The bypass option can be founded from system register and the bit >> of the option is 11. >> >> Cc: Inki Dae <inki.dae@samsung.com> >> Cc: Joonyoung Shim <jy0922.shim@samsung.com> >> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >> Signed-off-by: Chanho Park <chanho61.park@samsung.com> >> --- >> .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >> index 27c3ce0..7f90c4a 100644 >> --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >> +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >> @@ -45,6 +45,8 @@ Optional Properties: >> Can be used in case timings cannot be provided otherwise >> or to override timings provided by the panel. >> - samsung,sysreg: handle to syscon used to control the system registers >> +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. >> + This option is only available since exynos5420. >> - i80-if-timings: timing configuration for lcd i80 interface support. >> - cs-setup: clock cycles for the active period of address signal is enabled >> until chip select is enabled. >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index 70194d0..4fb2952 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -94,6 +94,7 @@ struct fimd_driver_data { >> unsigned int lcdblk_offset; >> unsigned int lcdblk_vt_shift; >> unsigned int lcdblk_bypass_shift; >> + unsigned int lcdblk_mic_bypass_shift; >> >> unsigned int has_shadowcon:1; >> unsigned int has_clksel:1; >> @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >> .lcdblk_offset = 0x214, >> .lcdblk_vt_shift = 24, >> .lcdblk_bypass_shift = 15, >> + .lcdblk_mic_bypass_shift = 11, >> .has_shadowcon = 1, >> .has_vidoutcon = 1, >> .has_vtsel = 1, >> @@ -162,6 +164,7 @@ struct fimd_context { >> u32 i80ifcon; >> bool i80_if; >> bool suspended; >> + bool mic_bypass; >> int pipe; >> wait_queue_head_t wait_vsync_queue; >> atomic_t wait_vsync_event; >> @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) >> return; >> } >> >> + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >> + driver_data->lcdblk_offset, >> + 0x1 << driver_data->lcdblk_mic_bypass_shift, >> + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >> + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); >> + return; >> + } > > It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. Actually, I wanted to set the bit on kernel side even though it's not assigned from bootloader. If the bootloader already set the bit, that means mic will be by-pass and we don't care about it from kernel side. The option is useful when I want to skip the mic on the kernel side. > > For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, > if (ctx->lcdblk_mic_bypass_shift) { It causes all exynos5 fimd driver skips the mic from display path. How about below patch instead of this? + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass") && + ctx->driver_data->lcdblk_mic_bypass_shift) + ctx->mic_bypass = true; Best Regards, Chanho Park > if (ctx->sysreg && regmap_update_bits(ctx->sysreg, > driver_data->lcdblk_offset, > ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift, > ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift)) { > ... > } > } > > Thanks, > Inki Dae > >> + >> /* setup horizontal and vertical display size. */ >> val = VIDTCON2_LINEVAL(mode->vdisplay - 1) | >> VIDTCON2_HOZVAL(mode->hdisplay - 1) | >> @@ -1014,6 +1025,9 @@ static int fimd_probe(struct platform_device *pdev) >> if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) >> ctx->vidcon1 |= VIDCON1_INV_VCLK; >> >> + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass")) >> + ctx->mic_bypass = true; >> + >> i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings"); >> if (i80_if_timings) { >> u32 val; >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Chanho, There was a missing one so below is my review again. 2016? 02? 11? 22:59? Chanho Park ?(?) ? ?: > Hi Inki, > > On Thu, Feb 11, 2016 at 7:37 PM, Inki Dae <inki.dae@samsung.com> wrote: >> Hi Chanho, >> >> 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: >>> From: Chanho Park <chanho61.park@samsung.com> >>> >>> This patch adds a mic_bypass option to bypass the mic >>> from display out path. The mic(Mobile image compressor) compresses >>> RGB data from fimd and send the compressed data to the mipi dsi. >>> The bypass option can be founded from system register and the bit >>> of the option is 11. >>> >>> Cc: Inki Dae <inki.dae@samsung.com> >>> Cc: Joonyoung Shim <jy0922.shim@samsung.com> >>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >>> Signed-off-by: Chanho Park <chanho61.park@samsung.com> >>> --- >>> .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ >>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ >>> 2 files changed, 16 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>> index 27c3ce0..7f90c4a 100644 >>> --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>> +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>> @@ -45,6 +45,8 @@ Optional Properties: >>> Can be used in case timings cannot be provided otherwise >>> or to override timings provided by the panel. >>> - samsung,sysreg: handle to syscon used to control the system registers >>> +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. mic-bypass is not really common property for fimd family so it's not correct for fimd device node has mic-bypass property. >>> + This option is only available since exynos5420. >>> - i80-if-timings: timing configuration for lcd i80 interface support. >>> - cs-setup: clock cycles for the active period of address signal is enabled >>> until chip select is enabled. >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> index 70194d0..4fb2952 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> @@ -94,6 +94,7 @@ struct fimd_driver_data { >>> unsigned int lcdblk_offset; >>> unsigned int lcdblk_vt_shift; >>> unsigned int lcdblk_bypass_shift; >>> + unsigned int lcdblk_mic_bypass_shift; >>> >>> unsigned int has_shadowcon:1; >>> unsigned int has_clksel:1; >>> @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >>> .lcdblk_offset = 0x214, >>> .lcdblk_vt_shift = 24, >>> .lcdblk_bypass_shift = 15, >>> + .lcdblk_mic_bypass_shift = 11, >>> .has_shadowcon = 1, >>> .has_vidoutcon = 1, >>> .has_vtsel = 1, >>> @@ -162,6 +164,7 @@ struct fimd_context { >>> u32 i80ifcon; >>> bool i80_if; >>> bool suspended; >>> + bool mic_bypass; >>> int pipe; >>> wait_queue_head_t wait_vsync_queue; >>> atomic_t wait_vsync_event; >>> @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) >>> return; >>> } >>> >>> + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >>> + driver_data->lcdblk_offset, >>> + 0x1 << driver_data->lcdblk_mic_bypass_shift, >>> + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >>> + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); >>> + return; >>> + } >> >> It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. > > Actually, I wanted to set the bit on kernel side even though it's not > assigned from bootloader. > If the bootloader already set the bit, that means mic will be by-pass > and we don't care about it from kernel side. > The option is useful when I want to skip the mic on the kernel side. > >> >> For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, >> if (ctx->lcdblk_mic_bypass_shift) { > > It causes all exynos5 fimd driver skips the mic from display path. > How about below patch instead of this? > > + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass") && > + ctx->driver_data->lcdblk_mic_bypass_shift) > + ctx->mic_bypass = true; So let's bypass mic path in default like lcdblk_bypass, /* set mic bypass selection */ if (ctx->has_mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, driver_data->lcdblk_offset, 0x1 << driver_data->lcdblk_mic_bypass_shift, 0x1 << driver_data->lcdblk_mic_bypass_shift)) { DRM_ERROR("Failed to update sysreg for mic bypass setting.\n"); return; } For this, I added a new member, has_mic_by_pass, which means that lcdblk register has mic_bypass bit like has_vtsel. We could consider display path such as mic and mDNIe later using of_graph concept below, Documentation/devicetree/bindings/graph.txt Thanks, Inki Dae > > Best Regards, > Chanho Park > >> if (ctx->sysreg && regmap_update_bits(ctx->sysreg, >> driver_data->lcdblk_offset, >> ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift, >> ctx->mic_bypass << driver_data->lcdblk_mic_bypass_shift)) { >> ... >> } >> } >> >> Thanks, >> Inki Dae >> >>> + >>> /* setup horizontal and vertical display size. */ >>> val = VIDTCON2_LINEVAL(mode->vdisplay - 1) | >>> VIDTCON2_HOZVAL(mode->hdisplay - 1) | >>> @@ -1014,6 +1025,9 @@ static int fimd_probe(struct platform_device *pdev) >>> if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) >>> ctx->vidcon1 |= VIDCON1_INV_VCLK; >>> >>> + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass")) >>> + ctx->mic_bypass = true; >>> + >>> i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings"); >>> if (i80_if_timings) { >>> u32 val; >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Feb 12, 2016 at 1:56 PM, Inki Dae <inki.dae@samsung.com> wrote: > Hi Chanho, > > There was a missing one so below is my review again. > > 2016? 02? 11? 22:59? Chanho Park ?(?) ? ?: >> Hi Inki, >> >> On Thu, Feb 11, 2016 at 7:37 PM, Inki Dae <inki.dae@samsung.com> wrote: >>> Hi Chanho, >>> >>> 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: >>>> From: Chanho Park <chanho61.park@samsung.com> >>>> >>>> This patch adds a mic_bypass option to bypass the mic >>>> from display out path. The mic(Mobile image compressor) compresses >>>> RGB data from fimd and send the compressed data to the mipi dsi. >>>> The bypass option can be founded from system register and the bit >>>> of the option is 11. >>>> >>>> Cc: Inki Dae <inki.dae@samsung.com> >>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >>>> Signed-off-by: Chanho Park <chanho61.park@samsung.com> >>>> --- >>>> .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ >>>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ >>>> 2 files changed, 16 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>> index 27c3ce0..7f90c4a 100644 >>>> --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>> +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>> @@ -45,6 +45,8 @@ Optional Properties: >>>> Can be used in case timings cannot be provided otherwise >>>> or to override timings provided by the panel. >>>> - samsung,sysreg: handle to syscon used to control the system registers >>>> +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. > > mic-bypass is not really common property for fimd family so it's not correct for fimd device node has mic-bypass property. According to user manual of exynos, the option has been introduced since exynos5420. The exynos5420 and exynos5422(5800) have a fimd controller not decon display controller. > >>>> + This option is only available since exynos5420. >>>> - i80-if-timings: timing configuration for lcd i80 interface support. >>>> - cs-setup: clock cycles for the active period of address signal is enabled >>>> until chip select is enabled. >>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> index 70194d0..4fb2952 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> @@ -94,6 +94,7 @@ struct fimd_driver_data { >>>> unsigned int lcdblk_offset; >>>> unsigned int lcdblk_vt_shift; >>>> unsigned int lcdblk_bypass_shift; >>>> + unsigned int lcdblk_mic_bypass_shift; >>>> >>>> unsigned int has_shadowcon:1; >>>> unsigned int has_clksel:1; >>>> @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >>>> .lcdblk_offset = 0x214, >>>> .lcdblk_vt_shift = 24, >>>> .lcdblk_bypass_shift = 15, >>>> + .lcdblk_mic_bypass_shift = 11, >>>> .has_shadowcon = 1, >>>> .has_vidoutcon = 1, >>>> .has_vtsel = 1, >>>> @@ -162,6 +164,7 @@ struct fimd_context { >>>> u32 i80ifcon; >>>> bool i80_if; >>>> bool suspended; >>>> + bool mic_bypass; >>>> int pipe; >>>> wait_queue_head_t wait_vsync_queue; >>>> atomic_t wait_vsync_event; >>>> @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) >>>> return; >>>> } >>>> >>>> + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >>>> + driver_data->lcdblk_offset, >>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift, >>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >>>> + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); >>>> + return; >>>> + } >>> >>> It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. >> >> Actually, I wanted to set the bit on kernel side even though it's not >> assigned from bootloader. >> If the bootloader already set the bit, that means mic will be by-pass >> and we don't care about it from kernel side. >> The option is useful when I want to skip the mic on the kernel side. >> >>> >>> For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, >>> if (ctx->lcdblk_mic_bypass_shift) { >> >> It causes all exynos5 fimd driver skips the mic from display path. >> How about below patch instead of this? >> >> + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass") && >> + ctx->driver_data->lcdblk_mic_bypass_shift) >> + ctx->mic_bypass = true; > > So let's bypass mic path in default like lcdblk_bypass, I'm not sure why the option is default for exynos542x fimd controller. The reset value of the bit is 0. That means the mic bypass is not a default option on chip-level. > /* set mic bypass selection */ > if (ctx->has_mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, > driver_data->lcdblk_offset, > 0x1 << driver_data->lcdblk_mic_bypass_shift, > 0x1 << driver_data->lcdblk_mic_bypass_shift)) { > DRM_ERROR("Failed to update sysreg for mic bypass setting.\n"); > return; > } > > For this, I added a new member, has_mic_by_pass, which means that lcdblk register has mic_bypass bit like has_vtsel. Hmm. Like lcd_blk_bypass_shift, the option is configurable because mic_by_pass_shift is not zero. I think we don't need the new has_mic_bypass option. > > We could consider display path such as mic and mDNIe later using of_graph concept below, > Documentation/devicetree/bindings/graph.txt Yes. To use the mic on display path, we'll define the graph in the device tree. In case of not using the mic, however, we'll not define any graphs in the dts. I intended this patch is just providing configurable option from device tree to skip mic from display path.
Hi Chanho, 2016? 02? 12? 16:39? Chanho Park ?(?) ? ?: > Hi, > > On Fri, Feb 12, 2016 at 1:56 PM, Inki Dae <inki.dae@samsung.com> wrote: >> Hi Chanho, >> >> There was a missing one so below is my review again. >> >> 2016? 02? 11? 22:59? Chanho Park ?(?) ? ?: >>> Hi Inki, >>> >>> On Thu, Feb 11, 2016 at 7:37 PM, Inki Dae <inki.dae@samsung.com> wrote: >>>> Hi Chanho, >>>> >>>> 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: >>>>> From: Chanho Park <chanho61.park@samsung.com> >>>>> >>>>> This patch adds a mic_bypass option to bypass the mic >>>>> from display out path. The mic(Mobile image compressor) compresses >>>>> RGB data from fimd and send the compressed data to the mipi dsi. >>>>> The bypass option can be founded from system register and the bit >>>>> of the option is 11. >>>>> >>>>> Cc: Inki Dae <inki.dae@samsung.com> >>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com> >>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >>>>> Signed-off-by: Chanho Park <chanho61.park@samsung.com> >>>>> --- >>>>> .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ >>>>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ >>>>> 2 files changed, 16 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>> index 27c3ce0..7f90c4a 100644 >>>>> --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>> +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>> @@ -45,6 +45,8 @@ Optional Properties: >>>>> Can be used in case timings cannot be provided otherwise >>>>> or to override timings provided by the panel. >>>>> - samsung,sysreg: handle to syscon used to control the system registers >>>>> +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. >> >> mic-bypass is not really common property for fimd family so it's not correct for fimd device node has mic-bypass property. > > According to user manual of exynos, the option has been introduced > since exynos5420. That is why samsung-fimd.txt is not correct place. samsung-fimd.txt describes hardware information for s3c24xx, s3c64xx also, which don't have mic ip. > The exynos5420 and exynos5422(5800) have a fimd controller not decon > display controller. > >> >>>>> + This option is only available since exynos5420. >>>>> - i80-if-timings: timing configuration for lcd i80 interface support. >>>>> - cs-setup: clock cycles for the active period of address signal is enabled >>>>> until chip select is enabled. >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>> index 70194d0..4fb2952 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>> @@ -94,6 +94,7 @@ struct fimd_driver_data { >>>>> unsigned int lcdblk_offset; >>>>> unsigned int lcdblk_vt_shift; >>>>> unsigned int lcdblk_bypass_shift; >>>>> + unsigned int lcdblk_mic_bypass_shift; >>>>> >>>>> unsigned int has_shadowcon:1; >>>>> unsigned int has_clksel:1; >>>>> @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >>>>> .lcdblk_offset = 0x214, >>>>> .lcdblk_vt_shift = 24, >>>>> .lcdblk_bypass_shift = 15, >>>>> + .lcdblk_mic_bypass_shift = 11, >>>>> .has_shadowcon = 1, >>>>> .has_vidoutcon = 1, >>>>> .has_vtsel = 1, >>>>> @@ -162,6 +164,7 @@ struct fimd_context { >>>>> u32 i80ifcon; >>>>> bool i80_if; >>>>> bool suspended; >>>>> + bool mic_bypass; >>>>> int pipe; >>>>> wait_queue_head_t wait_vsync_queue; >>>>> atomic_t wait_vsync_event; >>>>> @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) >>>>> return; >>>>> } >>>>> >>>>> + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >>>>> + driver_data->lcdblk_offset, >>>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift, >>>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >>>>> + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); >>>>> + return; >>>>> + } >>>> >>>> It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. >>> >>> Actually, I wanted to set the bit on kernel side even though it's not >>> assigned from bootloader. >>> If the bootloader already set the bit, that means mic will be by-pass >>> and we don't care about it from kernel side. >>> The option is useful when I want to skip the mic on the kernel side. >>> >>>> >>>> For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, >>>> if (ctx->lcdblk_mic_bypass_shift) { >>> >>> It causes all exynos5 fimd driver skips the mic from display path. >>> How about below patch instead of this? >>> >>> + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass") && >>> + ctx->driver_data->lcdblk_mic_bypass_shift) >>> + ctx->mic_bypass = true; >> >> So let's bypass mic path in default like lcdblk_bypass, > > I'm not sure why the option is default for exynos542x fimd controller. > The reset value of the bit is 0. That means the mic bypass is not a > default option on chip-level. That is because Exynos drm is not ready for mic support fully. As of now, Exynos drm uses mic driver only for Exynos5433 SoC but not other SoC. Do you know that? FIMDBYPASS bit of lcdblk register has 0 - Reserved not FIMD bypass(in case of Exynos5422) in default. > >> /* set mic bypass selection */ >> if (ctx->has_mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >> driver_data->lcdblk_offset, >> 0x1 << driver_data->lcdblk_mic_bypass_shift, >> 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >> DRM_ERROR("Failed to update sysreg for mic bypass setting.\n"); >> return; >> } >> >> For this, I added a new member, has_mic_by_pass, which means that lcdblk register has mic_bypass bit like has_vtsel. > Hmm. Like lcd_blk_bypass_shift, the option is configurable because > mic_by_pass_shift is not zero. > I think we don't need the new has_mic_bypass option. First of all, it seems that adding mic_bypass property to fimd device node is really wrong. If so, how can you let fimd driver know whether now SoC has mic bypass bit? > >> >> We could consider display path such as mic and mDNIe later using of_graph concept below, >> Documentation/devicetree/bindings/graph.txt > > Yes. To use the mic on display path, we'll define the graph in the device tree. > In case of not using the mic, however, we'll not define any graphs in the dts. However, fimd driver should know whether now SoC has mic bypass bit because fimd driver is used for other SoC also, which have no mic ip. Thanks, Inki Dae > > I intended this patch is just providing configurable option from > device tree to skip mic from display path. > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Feb 12, 2016 at 5:04 PM, Inki Dae <inki.dae@samsung.com> wrote: > Hi Chanho, > > 2016? 02? 12? 16:39? Chanho Park ?(?) ? ?: >> Hi, >> >> On Fri, Feb 12, 2016 at 1:56 PM, Inki Dae <inki.dae@samsung.com> wrote: >>> Hi Chanho, >>> >>> There was a missing one so below is my review again. >>> >>> 2016? 02? 11? 22:59? Chanho Park ?(?) ? ?: >>>> Hi Inki, >>>> >>>> On Thu, Feb 11, 2016 at 7:37 PM, Inki Dae <inki.dae@samsung.com> wrote: >>>>> Hi Chanho, >>>>> >>>>> 2016? 01? 30? 22:58? Chanho Park ?(?) ? ?: >>>>>> From: Chanho Park <chanho61.park@samsung.com> >>>>>> >>>>>> This patch adds a mic_bypass option to bypass the mic >>>>>> from display out path. The mic(Mobile image compressor) compresses >>>>>> RGB data from fimd and send the compressed data to the mipi dsi. >>>>>> The bypass option can be founded from system register and the bit >>>>>> of the option is 11. >>>>>> >>>>>> Cc: Inki Dae <inki.dae@samsung.com> >>>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com> >>>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >>>>>> Signed-off-by: Chanho Park <chanho61.park@samsung.com> >>>>>> --- >>>>>> .../devicetree/bindings/display/exynos/samsung-fimd.txt | 2 ++ >>>>>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++++ >>>>>> 2 files changed, 16 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>>> index 27c3ce0..7f90c4a 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>>> +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt >>>>>> @@ -45,6 +45,8 @@ Optional Properties: >>>>>> Can be used in case timings cannot be provided otherwise >>>>>> or to override timings provided by the panel. >>>>>> - samsung,sysreg: handle to syscon used to control the system registers >>>>>> +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. >>> >>> mic-bypass is not really common property for fimd family so it's not correct for fimd device node has mic-bypass property. >> >> According to user manual of exynos, the option has been introduced >> since exynos5420. > > That is why samsung-fimd.txt is not correct place. samsung-fimd.txt describes hardware information for s3c24xx, s3c64xx also, which don't have mic ip. Yes. I just described the option's limitation like below. > >> The exynos5420 and exynos5422(5800) have a fimd controller not decon >> display controller. >> >>> >>>>>> + This option is only available since exynos5420. I described the limitation in here. >>>>>> - i80-if-timings: timing configuration for lcd i80 interface support. >>>>>> - cs-setup: clock cycles for the active period of address signal is enabled >>>>>> until chip select is enabled. >>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>>> index 70194d0..4fb2952 100644 >>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>>>> @@ -94,6 +94,7 @@ struct fimd_driver_data { >>>>>> unsigned int lcdblk_offset; >>>>>> unsigned int lcdblk_vt_shift; >>>>>> unsigned int lcdblk_bypass_shift; >>>>>> + unsigned int lcdblk_mic_bypass_shift; >>>>>> >>>>>> unsigned int has_shadowcon:1; >>>>>> unsigned int has_clksel:1; >>>>>> @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >>>>>> .lcdblk_offset = 0x214, >>>>>> .lcdblk_vt_shift = 24, >>>>>> .lcdblk_bypass_shift = 15, >>>>>> + .lcdblk_mic_bypass_shift = 11, >>>>>> .has_shadowcon = 1, >>>>>> .has_vidoutcon = 1, >>>>>> .has_vtsel = 1, >>>>>> @@ -162,6 +164,7 @@ struct fimd_context { >>>>>> u32 i80ifcon; >>>>>> bool i80_if; >>>>>> bool suspended; >>>>>> + bool mic_bypass; >>>>>> int pipe; >>>>>> wait_queue_head_t wait_vsync_queue; >>>>>> atomic_t wait_vsync_event; >>>>>> @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) >>>>>> return; >>>>>> } >>>>>> >>>>>> + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >>>>>> + driver_data->lcdblk_offset, >>>>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift, >>>>>> + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >>>>>> + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); >>>>>> + return; >>>>>> + } >>>>> >>>>> It'd better to consider mic path also because mic bypass bit of lcdblk could be true by bootloader. In this case, fimd driver wouldn't do anything if mic_bypass property isn't declared but the mic_bypass bit of lcdblk register is still set to 1. >>>> >>>> Actually, I wanted to set the bit on kernel side even though it's not >>>> assigned from bootloader. >>>> If the bootloader already set the bit, that means mic will be by-pass >>>> and we don't care about it from kernel side. >>>> The option is useful when I want to skip the mic on the kernel side. >>>> >>>>> >>>>> For this, I think you could check lcdblk_mic_pypass_shift to identify whether mic path is supported or not like below, >>>>> if (ctx->lcdblk_mic_bypass_shift) { >>>> >>>> It causes all exynos5 fimd driver skips the mic from display path. >>>> How about below patch instead of this? >>>> >>>> + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass") && >>>> + ctx->driver_data->lcdblk_mic_bypass_shift) >>>> + ctx->mic_bypass = true; >>> >>> So let's bypass mic path in default like lcdblk_bypass, >> >> I'm not sure why the option is default for exynos542x fimd controller. >> The reset value of the bit is 0. That means the mic bypass is not a >> default option on chip-level. > > That is because Exynos drm is not ready for mic support fully. As of now, Exynos drm uses mic driver only for Exynos5433 SoC but not other SoC. The MIC is required to process 2560x1440 large image. I think it's not necessary under full-hd size image. So, the option is configurable not fixed during display path. I'll also try to enable the option in exynos5422. > Do you know that? FIMDBYPASS bit of lcdblk register has 0 - Reserved not FIMD bypass(in case of Exynos5422) in default. > >> >>> /* set mic bypass selection */ >>> if (ctx->has_mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, >>> driver_data->lcdblk_offset, >>> 0x1 << driver_data->lcdblk_mic_bypass_shift, >>> 0x1 << driver_data->lcdblk_mic_bypass_shift)) { >>> DRM_ERROR("Failed to update sysreg for mic bypass setting.\n"); >>> return; >>> } >>> >>> For this, I added a new member, has_mic_by_pass, which means that lcdblk register has mic_bypass bit like has_vtsel. >> Hmm. Like lcd_blk_bypass_shift, the option is configurable because >> mic_by_pass_shift is not zero. >> I think we don't need the new has_mic_bypass option. > > First of all, it seems that adding mic_bypass property to fimd device node is really wrong. If so, how can you let fimd driver know whether now SoC has mic bypass bit? Yes. To complete this patch, we should split exynos5420 from exynos5. Actually, I tried but I was not willing to do that only this bit. > >> >>> >>> We could consider display path such as mic and mDNIe later using of_graph concept below, >>> Documentation/devicetree/bindings/graph.txt >> >> Yes. To use the mic on display path, we'll define the graph in the device tree. >> In case of not using the mic, however, we'll not define any graphs in the dts. > > However, fimd driver should know whether now SoC has mic bypass bit because fimd driver is used for other SoC also, which have no mic ip. If you want, I'll make new patch series to support exynos5420 of fimd.
diff --git a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt index 27c3ce0..7f90c4a 100644 --- a/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/display/exynos/samsung-fimd.txt @@ -45,6 +45,8 @@ Optional Properties: Can be used in case timings cannot be provided otherwise or to override timings provided by the panel. - samsung,sysreg: handle to syscon used to control the system registers +- samsung,mic-bypass: bypass mic(mobile image compressor) from display path. + This option is only available since exynos5420. - i80-if-timings: timing configuration for lcd i80 interface support. - cs-setup: clock cycles for the active period of address signal is enabled until chip select is enabled. diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 70194d0..4fb2952 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -94,6 +94,7 @@ struct fimd_driver_data { unsigned int lcdblk_offset; unsigned int lcdblk_vt_shift; unsigned int lcdblk_bypass_shift; + unsigned int lcdblk_mic_bypass_shift; unsigned int has_shadowcon:1; unsigned int has_clksel:1; @@ -140,6 +141,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { .lcdblk_offset = 0x214, .lcdblk_vt_shift = 24, .lcdblk_bypass_shift = 15, + .lcdblk_mic_bypass_shift = 11, .has_shadowcon = 1, .has_vidoutcon = 1, .has_vtsel = 1, @@ -162,6 +164,7 @@ struct fimd_context { u32 i80ifcon; bool i80_if; bool suspended; + bool mic_bypass; int pipe; wait_queue_head_t wait_vsync_queue; atomic_t wait_vsync_event; @@ -461,6 +464,14 @@ static void fimd_commit(struct exynos_drm_crtc *crtc) return; } + if (ctx->mic_bypass && ctx->sysreg && regmap_update_bits(ctx->sysreg, + driver_data->lcdblk_offset, + 0x1 << driver_data->lcdblk_mic_bypass_shift, + 0x1 << driver_data->lcdblk_mic_bypass_shift)) { + DRM_ERROR("Failed to update sysreg for bypass mic.\n"); + return; + } + /* setup horizontal and vertical display size. */ val = VIDTCON2_LINEVAL(mode->vdisplay - 1) | VIDTCON2_HOZVAL(mode->hdisplay - 1) | @@ -1014,6 +1025,9 @@ static int fimd_probe(struct platform_device *pdev) if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK; + if (of_property_read_bool(dev->of_node, "samsung,mic-bypass")) + ctx->mic_bypass = true; + i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings"); if (i80_if_timings) { u32 val;