[RFC] drm/exynos: abort commit when framebuffer is removed from plane
On 8 July 2014 21:25, Inki Dae wrote: > 2014-06-20 0:13 GMT+09:00 Rahul Sharma : >> This situation arises when userspace remove the frambuffer object >> and call setmode ioctl. >> >> drm_mode_rmfb --> drm_plane_force_disable --> plane->crtc = NULL; >> and >> drm_mode_setcrtc --> exynos_plane_commit --> passes plane->crtc to >> exynos_drm_crtc_plane_commit which is NULL. > > If user process requested drm_mode_rmfb with a fb_id, fb object to the > fb_id must be removed from crtc_idr table. So drm_mode_setcrtc should > be failed because there is no the fb object in the crtc_idr table > anymore. > I cannot understand how exynos_drm_crtc_plane_commit function could be > called. Can you give me more details? Inki, These logs should clarify more about the problem: localhost ~ # halt localhost ~ # [ 130.570309] init: debugd main process (781) killed by TERM signal [ 130.602453] init: lid_touchpad_helper main process (2100) killed by TERM signal [ 131.374955] CPU: 2 PID: 834 Comm: X Tainted: GW 3.16.0-rc1+ #623 [ 131.380558] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 131.388327] [] (show_stack) from [] (dump_stack+0x7c/0x98) [ 131.395522] [] (dump_stack) from [] (exynos_drm_crtc_plane_commit+0x20/0x40) [ 131.404263] [] (exynos_drm_crtc_plane_commit) from [] (exynos_plane_commit+0x24/0x28) [ 131.413779] [] (exynos_plane_commit) from [] (exynos_drm_crtc_commit+0x2c/0x54) [ 131.422802] [] (exynos_drm_crtc_commit) from [] (exynos_drm_crtc_mode_set_commit.isra.1+0x8c/0xa0) [ 131.433468] [] (exynos_drm_crtc_mode_set_commit.isra.1) from [] (exynos_drm_crtc_page_flip+0x100/0x174) [ 131.444587] [] (exynos_drm_crtc_page_flip) from [] (drm_mode_page_flip_ioctl+0x1f0/0x2b0) -->> [ 131.454460] [] (drm_mode_page_flip_ioctl) from [] (drm_ioctl+0x270/0x44c) [ 131.462966] [] (drm_ioctl) from [] (do_vfs_ioctl+0x4e4/0x5a0) [ 131.470397] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x5c/0x84) [ 131.477728] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30) [ 131.762797] CPU: 1 PID: 834 Comm: X Tainted: GW 3.16.0-rc1+ #623 [ 131.768378] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 131.776151] [] (show_stack) from [] (dump_stack+0x7c/0x98) [ 131.783315] [] (dump_stack) from [] (drm_plane_force_disable+0x5c/0x68) [ 131.791658] [] (drm_plane_force_disable) from [] (drm_framebuffer_remove+0xe4/0x110) [ 131.801070] [] (drm_framebuffer_remove) from [] (drm_mode_rmfb+0xd4/0xfc) -->> [ 131.809597] [] (drm_mode_rmfb) from [] (drm_ioctl+0x270/0x44c) [ 131.817135] [] (drm_ioctl) from [] (do_vfs_ioctl+0x4e4/0x5a0) [ 131.824609] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x5c/0x84) [ 131.831884] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30) [ 132.077803] CPU: 0 PID: 834 Comm: X Tainted: GW 3.16.0-rc1+ #623 [ 132.083413] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 132.09] [] (show_stack) from [] (dump_stack+0x7c/0x98) [ 132.098343] [] (dump_stack) from [] (exynos_drm_crtc_plane_commit+0x20/0x40) [ 132.107098] [] (exynos_drm_crtc_plane_commit) from [] (exynos_plane_commit+0x24/0x28) [ 132.116631] [] (exynos_plane_commit) from [] (exynos_drm_crtc_commit+0x2c/0x54) [ 132.125660] [] (exynos_drm_crtc_commit) from [] (exynos_drm_crtc_mode_set_commit.isra.1+0x8c/0xa0) [ 132.136330] [] (exynos_drm_crtc_mode_set_commit.isra.1) from [] (exynos_drm_crtc_mode_set_base+0x18/0x1c) [ 132.147605] [] (exynos_drm_crtc_mode_set_base) from [] (drm_crtc_helper_set_config+0x828/0x8a4) [ 132.158029] [] (drm_crtc_helper_set_config) from [] (drm_mode_set_config_internal+0x58/0xc0) [ 132.168155] [] (drm_mode_set_config_internal) from [] (drm_mode_setcrtc+0x388/0x4ac) -->> [ 132.177630] [] (drm_mode_setcrtc) from [] (drm_ioctl+0x270/0x44c) [ 132.185417] [] (drm_ioctl) from [] (do_vfs_ioctl+0x4e4/0x5a0) [ 132.192897] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x5c/0x84) [ 132.200138] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30) [ 132.207735] Unable to handle kernel NULL pointer dereference at virtual address 032c .. .. [ 132.510786] ff80: b6ebdeb8 bee1d5e8 c06864a2 0036 c000e5a4 ecf0e000 ecf0ffa8 [ 132.518941] ffa0: c000e380 c01130f4 b6ebdeb8 bee1d5e8 0005 c06864a2 bee1d5e8 0001 [ 132.527095] ffc0: b6ebdeb8 bee1d5e8 c06864a2 0036 b85d4a74 b8702a60 bee1d688 [ 132.535250] ffe0: b6a82f30 bee1d5cc b6a75cff b6bce50c 0010 0005 e1a0c00d e92dd800 [ 132.543408] [] (exynos_drm_crtc_plane_commit) from [] (exynos_plane_commit+0x24/0x28) [ 132.552949] [] (exynos_plane_commit) from [] (exynos_drm_crtc_commit+0x2c/0x54) [ 132.561971] [] (exynos_drm_crtc_commit) from [] (exynos_drm_crtc_mode_set_commit.isra.1+0x8c/0xa0) [ 132.572641] [] (exynos_drm_crtc_mode_set_commit.isra.1) from [] (exynos_drm_crtc_mode_set_base+0x18/0x1c) [ 132.583919] [] (exynos_drm_crtc_mode_set_base) from [] (drm_crtc_helper_set_config+0x828/0x8a4) [ 132.
[RFC] drm/exynos: abort commit when framebuffer is removed from plane
Hi Inki,
What do you think about the following fix? I need your inputs for this.
Regards,
Rahul Sharma
On 19 June 2014 20:43, Rahul Sharma wrote:
> This situation arises when userspace remove the frambuffer object
> and call setmode ioctl.
>
> drm_mode_rmfb --> drm_plane_force_disable --> plane->crtc = NULL;
> and
> drm_mode_setcrtc --> exynos_plane_commit --> passes plane->crtc to
> exynos_drm_crtc_plane_commit which is NULL.
>
> This crashes the system.
>
> Signed-off-by: Rahul Sharma
> ---
> This works fine but I am not confident on the correctness of the
> solution.
>
> drivers/gpu/drm/exynos/exynos_drm_crtc.c |6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 95c9435..da4efe4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -165,6 +165,12 @@ static int exynos_drm_crtc_mode_set_commit(struct
> drm_crtc *crtc, int x, int y,
> return -EPERM;
> }
>
> + /* when framebuffer is removed, commit should not proceed. */
> + if(!plane->fb){
> + DRM_ERROR("framebuffer has been removed from plane.\n");
> + return -EFAULT;
> + }
> +
> crtc_w = crtc->primary->fb->width - x;
> crtc_h = crtc->primary->fb->height - y;
>
> --
> 1.7.9.5
>
[PATCH 1/3] drm/exynos: Control DISP1BLK system register in FIMD
Hi Ajay,
On 25 June 2014 19:45, Ajay Kumar wrote:
> Exynos SOC have a DISP1BLK register where we can select
> the path for FIMD output. We can redirect the video data
> directly to DP/MIPI interface, or we can pass it via
> image enhancement chips.
>
> Since we don't use any image enhancement chips in exynos-drm,
> we need to set FIMD BYPASS in DISP1BLK.
>
> Signed-off-by: Ajay Kumar
> ---
> .../devicetree/bindings/video/samsung-fimd.txt |2 ++
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++
> include/video/samsung_fimd.h |5 +
> 3 files changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt
> b/Documentation/devicetree/bindings/video/samsung-fimd.txt
> index 2dad41b..034e3458 100644
> --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt
> +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt
> @@ -44,6 +44,8 @@ Optional Properties:
> - display-timings: timing settings for FIMD, as described in document [1].
> Can be used in case timings cannot be provided otherwise
> or to override timings provided by the panel.
> +- samsung,sysreg-phandle - handle to syscon node, used to control the system
> + registers for DISP1BLK.
>
> The device node can contain 'port' child nodes according to the bindings
> defined
> in [2]. The following are properties specific to those nodes:
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 33161ad..a3c855b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -20,6 +20,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
> #include
> @@ -68,6 +70,7 @@
>
> struct fimd_driver_data {
> unsigned int timing_base;
> + unsigned int sysreg_disp1blk_offset;
>
> unsigned int has_shadowcon:1;
> unsigned int has_clksel:1;
> @@ -83,11 +86,13 @@ static struct fimd_driver_data s3c64xx_fimd_driver_data =
> {
> static struct fimd_driver_data exynos4_fimd_driver_data = {
> .timing_base = 0x0,
> .has_shadowcon = 1,
> + .sysreg_disp1blk_offset = 0x210,
offset should be defined as Macros in reg file.
> };
>
> static struct fimd_driver_data exynos5_fimd_driver_data = {
> .timing_base = 0x2,
> .has_shadowcon = 1,
> + .sysreg_disp1blk_offset = 0x214,
> };
Same as above. Rest looks fine to me.
Regards,
Rahul Sharma.
>
> struct fimd_win_data {
> @@ -112,6 +117,7 @@ struct fimd_context {
> struct clk *bus_clk;
> struct clk *lcd_clk;
> void __iomem*regs;
> + struct regmap *sysreg;
> struct drm_display_mode mode;
> struct fimd_win_datawin_data[WINDOWS_NR];
> unsigned intdefault_win;
> @@ -760,6 +766,12 @@ static int fimd_poweron(struct exynos_drm_manager *mgr)
>
> pm_runtime_get_sync(ctx->dev);
>
> + if (ctx->sysreg)
> + regmap_update_bits(ctx->sysreg,
> + ctx->driver_data->sysreg_disp1blk_offset,
> + FIMDBYPASS_MASK,
> + FIMDBYPASS << FIMDBYPASS_SHIFT);
> +
> ret = clk_prepare_enable(ctx->bus_clk);
> if (ret < 0) {
> DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
> @@ -998,6 +1010,13 @@ static int fimd_probe(struct platform_device *pdev)
> if (IS_ERR(ctx->display))
> return PTR_ERR(ctx->display);
>
> + ctx->sysreg = syscon_regmap_lookup_by_phandle(
> + pdev->dev.of_node, "samsung,sysreg-phandle");
> + if (IS_ERR(ctx->sysreg)) {
> + DRM_INFO("DISP1BLK system register is not configured\n");
> + ctx->sysreg = NULL;
> + }
> +
> pm_runtime_enable(&pdev->dev);
>
> ret = component_add(&pdev->dev, &fimd_component_ops);
> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
> index b039320..b92267d 100644
> --- a/include/video/samsung_fimd.h
> +++ b/include/video/samsung_fimd.h
> @@ -462,3 +462,8 @@
> #define FIMD_V8_VIDTCON2 0x20018
> #define FIMD_V8_VIDTCON3 0x2001C
> #define FIMD_V8_VIDCON10x20004
> +
> +/* SYSREG DISP1BLK definitions */
> +#define FIMDBYPASS_SHIFT 15
> +#define FIMDBYPASS_MASK(1 << FIMDBYPASS_SHIFT)
> +#define FIMDBYPASS 1
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5 v2] drm/exynos: allow mulitple layer updates per vsync for mixer
Thanks Inki,
One more thing. mixer_layer_update is only called on for mixer version;
MXR_VER_16_0_33_0, MXR_VER_128_0_0_184. This condition
should have taken care of Exynos4 scenarios. What you say?
Regards,
Rahul Sharma.
On 24 June 2014 20:20, Inki Dae wrote:
> 2014-06-24 20:38 GMT+09:00 Andreas F?rber :
>> Am 24.06.2014 07:21, schrieb Inki Dae:
>>> On 2014? 06? 23? 14:32, Rahul Sharma wrote:
>>>> Allowing only one layer update per vsync can cause issues
>>>> while there are update available for both layers. There is
>>>> a good amount of possibility to loose updates if we allow
>>>> single update per vsync.
>>>>
>>>> Signed-off-by: Rahul Sharma
>>>> ---
>>>> drivers/gpu/drm/exynos/exynos_mixer.c |7 +--
>>>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> index d359501..6773b03 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context
>>>> *ctx, int win)
>>>> static void mixer_layer_update(struct mixer_context *ctx)
>>>> {
>>>> struct mixer_resources *res = &ctx->mixer_res;
>>>> -u32 val;
>>>> -
>>>> -val = mixer_reg_read(res, MXR_CFG);
>>>>
>>>> -/* allow one update per vsync only */
>>>> -if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
>>>> -mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>> +mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>
>>> Rahul, it looks good to me and ok as is. But above codes don't consider
>>> Exynos4 series. In case of Exynos4xxx SoC,
>>> MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
>>> register are reserved fields. So can you work that patch to be
>>> considered for Exynos4xxx SoC? That patch would be additional one.
>>>
>>> Anyway, will apply it as is, and I will wait for the additional patch.
>>
>> If it's not too late, could you fix up "multiple" in the subject? :)
>
> Corrected. :)
>
> Thanks,
> Inki Dae
>
>>
>> Cheers,
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
>> in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 00/10] drm: exynos: few patches to enhance bridge chip support
Hi Tomasz, On 23 June 2014 20:08, Tomasz Figa wrote: > Hi Rahul, > > On 23.06.2014 15:58, Rahul Sharma wrote: >> Hi Ajay, Inki, >> >> I tested this series for Exynos5420 based peach-pit board, >> Exynos5800 based Peach-pi board and Exynos5250 based >> Snow board. I verified with the chrome test environment and >> able to see upto Login Screen. DPMS on/off functionality and >> S2R is also working fine for Display. therefore: > > What tree did you apply this patches onto? I don't see S2R support for > Exynos5420 or 5800 in current linux-next (e.g. there are no PMU tables > present for these SoCs). I verified them with inflight patches and some private patches. All of them are available at https://github.com/exynos-reference/kernel/tree/linux-3.16-rc1-ui-s2r Regards, Rahul Sharma. > > Best regards, > Tomasz > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V4 00/10] drm: exynos: few patches to enhance bridge chip support
Hi Ajay, Inki, I tested this series for Exynos5420 based peach-pit board, Exynos5800 based Peach-pi board and Exynos5250 based Snow board. I verified with the chrome test environment and able to see upto Login Screen. DPMS on/off functionality and S2R is also working fine for Display. therefore: Tested-by: Rahul Sharma Regards, Rahul Sharma. On 20 June 2014 21:21, Inki Dae wrote: > 2014-06-20 17:06 GMT+09:00 Ajay kumar : >> ping. > > I will have a review soon but I'm afraid that I cannot have a test yet > because I have no any board with panel based on eDP and LVDS so wait > for until I get a board. > Otherwise, can anyone give me tested-by? and I'd happy to give me > reviewed-by so that I can pick this patch series up. > > Thanks, > Inki Dae > >> >> On Wed, Jun 11, 2014 at 11:56 PM, Ajay Kumar >> wrote: >>> This series is based on exynos-drm-next branch of Inki Dae's tree at: >>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >>> >>> I have tested this after adding few DT changes for exynos5250-snow, >>> exynos5420-peach-pit and exynos5800-peach-pi boards. >>> >>> This patchset also consolidates various inputs from the drm community >>> regarding the bridge chaining concept: >>> (1) [RFC V2 0/3] drm/bridge: panel and chaining >>> http://www.spinics.net/lists/linux-samsung-soc/msg30160.html >>> (2) [RFC V3 0/3] drm/bridge: panel and chaining >>> http://www.spinics.net/lists/linux-samsung-soc/msg30507.html >>> >>> Changes since V2: >>> -- Address comments from Jingoo Han for ps8622 driver >>> -- Address comments from Daniel, Rob and Thierry regarding >>>bridge chaining >>> -- Address comments from Thierry regarding the names for >>>new drm_panel functions >>> >>> Changes since V3: >>> -- Remove hotplug based initialization of exynos_dp >>> -- Make exynos_dp work directly with drm_panel, remove >>>dependency on panel_binder >>> -- Minor cleanups in panel_binder and panel_lvds driver >>> >>> The following patches can be divided into 2 groups: >>> patches 1 to 4: add drm_panel support to exynos_dp(peach-pi) >>> patches 5 to 10: chaining of bridges and drm_panel(snow and >>> peach-pit) >>> >>> Ajay Kumar (8): >>> [PATCH V4 1/10] drm/exynos: Move DP setup out of hotplug workqueue >>> [PATCH V4 2/10] drm/panel: add prepare and unprepare routines >>> [PATCH V4 3/10] drm/exynos: dp: modify driver to support drm_panel >>> [PATCH V4 4/10] drm/panel: Add driver for lvds/edp based panels >>> [PATCH V4 5/10] drm/bridge: add helper functions to support bridge chain >>> [PATCH V4 6/10] drm/bridge: Add a driver which binds drm_bridge with >>> drm_panel >>> [PATCH V4 7/10] drm/bridge: ptn3460: Support bridge chaining >>> [PATCH V4 8/10] drm/exynos: dp: create bridge chain using ptn3460 and >>> panel_binder >>> >>> Vincent Palatin (1): >>> [PATCH V4 9/10] drm/bridge: Add ps8622/ps8625 bridge driver >>> >>> Rahul Sharma (1): >>> [PATCH V4 10/10] drm/exynos: Add ps8622 lvds bridge discovery to DP driver >>> >>> .../devicetree/bindings/drm/bridge/ps8622.txt | 21 + >>> .../devicetree/bindings/panel/panel-lvds.txt | 50 +++ >>> .../devicetree/bindings/video/exynos_dp.txt|2 + >>> drivers/gpu/drm/bridge/Kconfig | 15 + >>> drivers/gpu/drm/bridge/Makefile|2 + >>> drivers/gpu/drm/bridge/panel_binder.c | 193 >>> drivers/gpu/drm/bridge/ps8622.c| 475 >>> >>> drivers/gpu/drm/bridge/ptn3460.c | 136 +- >>> drivers/gpu/drm/exynos/Kconfig |1 + >>> drivers/gpu/drm/exynos/exynos_dp_core.c| 87 +++- >>> drivers/gpu/drm/exynos/exynos_dp_core.h|2 + >>> drivers/gpu/drm/panel/Kconfig | 10 + >>> drivers/gpu/drm/panel/Makefile |1 + >>> drivers/gpu/drm/panel/panel-lvds.c | 262 +++ >>> include/drm/bridge/panel_binder.h | 44 ++ >>> include/drm/bridge/ps8622.h| 41 ++ >>> include/drm/bridge/ptn3460.h | 15 +- >>> include/drm/drm_crtc.h
[PATCH v2] drm/exynos: defer hdmi probe when fail to get regulators
HDMI probe proceeds with dummy regulators when the regulators
are not provided in DT node or regulator provider has not get
probed or failed to register the regulators.
This patch modify hdmi driver to defer the probe in case the
regulators are not available.
Signed-off-by: Rahul Sharma
---
v2: Return Error code from devm_regulator_get_optional (as it is).
Based on exynos-drm-fixes branch in Inki dae's tree.
drivers/gpu/drm/exynos/exynos_hdmi.c | 69 --
1 file changed, 50 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index c104d0c..d05da3b 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -83,7 +83,7 @@ struct hdmi_resources {
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
struct clk *mout_hdmi;
- struct regulator_bulk_data *regul_bulk;
+ struct regulator**regulators;
int regul_count;
};
@@ -2022,6 +2022,36 @@ static void hdmi_commit(struct exynos_drm_display
*display)
hdmi_conf_apply(hdata);
}
+int hdmi_regulator_enable(struct hdmi_context *hdata)
+{
+ struct hdmi_resources *res = &hdata->res;
+ int i, ret;
+
+ for (i = 0; i < res->regul_count; ++i) {
+ ret = regulator_enable(res->regulators[i]);
+ if (ret < 0) {
+ DRM_ERROR("fail to enable regulators.\n");
+ return ret;
+ }
+ }
+ return 0;
+}
+
+int hdmi_regulator_disable(struct hdmi_context *hdata)
+{
+ struct hdmi_resources *res = &hdata->res;
+ int i, ret;
+
+ for (i = 0; i < res->regul_count; ++i) {
+ ret = regulator_disable(res->regulators[i]);
+ if (ret < 0) {
+ DRM_ERROR("fail to disable regulators.\n");
+ return ret;
+ }
+ }
+ return 0;
+}
+
static void hdmi_poweron(struct exynos_drm_display *display)
{
struct hdmi_context *hdata = display->ctx;
@@ -2039,8 +2069,8 @@ static void hdmi_poweron(struct exynos_drm_display
*display)
pm_runtime_get_sync(hdata->dev);
- if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
- DRM_DEBUG_KMS("failed to enable regulator bulk\n");
+ if (hdmi_regulator_enable(hdata))
+ DRM_DEBUG_KMS("failed to enable regulators\n");
/* set pmu hdmiphy control bit to enable hdmiphy */
regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
@@ -2077,7 +2107,8 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
PMU_HDMI_PHY_ENABLE_BIT, 0);
- regulator_bulk_disable(res->regul_count, res->regul_bulk);
+ if (hdmi_regulator_disable(hdata))
+ DRM_DEBUG_KMS("failed to disable regulators\n");
pm_runtime_put_sync(hdata->dev);
@@ -2192,24 +2223,24 @@ static int hdmi_resources_init(struct hdmi_context
*hdata)
clk_set_parent(res->mout_hdmi, res->sclk_pixel);
- res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
- sizeof(res->regul_bulk[0]), GFP_KERNEL);
- if (!res->regul_bulk) {
- ret = -ENOMEM;
- goto fail;
- }
+ res->regul_count = ARRAY_SIZE(supply);
+
+ res->regulators = devm_kzalloc(dev, res->regul_count *
+ sizeof(res->regulators[0]), GFP_KERNEL);
+ if (!res->regulators)
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(supply); ++i) {
- res->regul_bulk[i].supply = supply[i];
- res->regul_bulk[i].consumer = NULL;
- }
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
- if (ret) {
- DRM_ERROR("failed to get regulators\n");
- return ret;
+ res->regulators[i] =
+ devm_regulator_get_optional(dev, supply[i]);
+ if (IS_ERR(res->regulators[i])) {
+ DRM_ERROR("fail to get regulator: %s.\n",
+ supply[i]);
+ return PTR_ERR(res->regulators[i]);
+ }
}
- res->regul_count = ARRAY_SIZE(supply);
- return ret;
+ return 0;
fail:
DRM_ERROR("HDMI resource init - failed\n");
return ret;
--
1.7.9.5
[PATCH 5/5 v2] drm/exynos: enable vsync interrupt while waiting for vblank
mixer_wait_for_vblank function expects that the upcoming
vsync interrupt handler routine will clear the
wait_vsync_event atomic variable.
For this to happen, interrupts should be enabled and
disabled properly.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c |4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 6f18581..7529946 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1019,6 +1019,8 @@ static void mixer_wait_for_vblank(struct
exynos_drm_manager *mgr)
}
mutex_unlock(&mixer_ctx->mixer_mutex);
+ drm_vblank_get(mgr->crtc->dev, mixer_ctx->pipe);
+
atomic_set(&mixer_ctx->wait_vsync_event, 1);
/*
@@ -1029,6 +1031,8 @@ static void mixer_wait_for_vblank(struct
exynos_drm_manager *mgr)
!atomic_read(&mixer_ctx->wait_vsync_event),
HZ/20))
DRM_DEBUG_KMS("vblank wait timed out.\n");
+
+ drm_vblank_put(mgr->crtc->dev, mixer_ctx->pipe);
}
static void mixer_window_suspend(struct exynos_drm_manager *mgr)
--
1.7.9.5
[PATCH 4/5 v2] drm/exynos: soft reset mixer before reconfigure after power-on
Mixer soft reset is a recommended step before reconfiguring the mixer after power on. Mixer looses the previous state of DMAs if soft reset. This is the recommendation from the hardware team. Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_mixer.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 6773b03..6f18581 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1085,6 +1085,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) ctx->powered = true; mutex_unlock(&ctx->mixer_mutex); + mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET); + mixer_reg_write(res, MXR_INT_EN, ctx->int_en); mixer_win_reset(ctx); -- 1.7.9.5
[PATCH 3/5 v2] drm/exynos: allow mulitple layer updates per vsync for mixer
Allowing only one layer update per vsync can cause issues
while there are update available for both layers. There is
a good amount of possibility to loose updates if we allow
single update per vsync.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c |7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index d359501..6773b03 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int
win)
static void mixer_layer_update(struct mixer_context *ctx)
{
struct mixer_resources *res = &ctx->mixer_res;
- u32 val;
-
- val = mixer_reg_read(res, MXR_CFG);
- /* allow one update per vsync only */
- if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
- mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
+ mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
}
static void mixer_graph_buffer(struct mixer_context *ctx, int win)
--
1.7.9.5
[PATCH 2/5 v2] drm/exynos: stop mixer before gating clocks during poweroff
Mixer should be power gated only after it is gracefully stopped.
The recommended sequence is to Stop the mixer and wait till
it enters to IDLE state before gating the clocks and power to
the mixer.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c | 15 +++
drivers/gpu/drm/exynos/regs-mixer.h |1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index c00abbe..d359501 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -377,6 +377,20 @@ static void mixer_run(struct mixer_context *ctx)
mixer_regs_dump(ctx);
}
+static void mixer_stop(struct mixer_context *ctx)
+{
+ struct mixer_resources *res = &ctx->mixer_res;
+ int timeout = 20;
+
+ mixer_reg_writemask(res, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
+
+ while (!(mixer_reg_read(res, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
+ --timeout)
+ usleep_range(1, 12000);
+
+ mixer_regs_dump(ctx);
+}
+
static void vp_video_buffer(struct mixer_context *ctx, int win)
{
struct mixer_resources *res = &ctx->mixer_res;
@@ -1094,6 +1108,7 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr)
}
mutex_unlock(&ctx->mixer_mutex);
+ mixer_stop(ctx);
mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
diff --git a/drivers/gpu/drm/exynos/regs-mixer.h
b/drivers/gpu/drm/exynos/regs-mixer.h
index 4537026..5f32e1a 100644
--- a/drivers/gpu/drm/exynos/regs-mixer.h
+++ b/drivers/gpu/drm/exynos/regs-mixer.h
@@ -78,6 +78,7 @@
#define MXR_STATUS_BIG_ENDIAN (1 << 3)
#define MXR_STATUS_ENDIAN_MASK (1 << 3)
#define MXR_STATUS_SYNC_ENABLE (1 << 2)
+#define MXR_STATUS_REG_IDLE(1 << 1)
#define MXR_STATUS_REG_RUN (1 << 0)
/* bits for MXR_CFG */
--
1.7.9.5
[PATCH 1/5 v2] drm/exynos: set power state variable after enabling clocks and power
Power state variable holds the state of the mixer device.
Power on and power off functions are toggling these variable
at wrong place.
State variable should be changed to true only after Runtime
PM and clocks are enabled. Else it may result to a situation
where mixer registers are accessed with device power enabled.
Similar logic for poweroff sequence.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4c5aed7..c00abbe 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1061,7 +1061,7 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
mutex_unlock(&ctx->mixer_mutex);
return;
}
- ctx->powered = true;
+
mutex_unlock(&ctx->mixer_mutex);
pm_runtime_get_sync(ctx->dev);
@@ -1072,6 +1072,10 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
clk_prepare_enable(res->sclk_mixer);
}
+ mutex_lock(&ctx->mixer_mutex);
+ ctx->powered = true;
+ mutex_unlock(&ctx->mixer_mutex);
+
mixer_reg_write(res, MXR_INT_EN, ctx->int_en);
mixer_win_reset(ctx);
@@ -1084,14 +1088,20 @@ static void mixer_poweroff(struct exynos_drm_manager
*mgr)
struct mixer_resources *res = &ctx->mixer_res;
mutex_lock(&ctx->mixer_mutex);
- if (!ctx->powered)
- goto out;
+ if (!ctx->powered) {
+ mutex_unlock(&ctx->mixer_mutex);
+ return;
+ }
mutex_unlock(&ctx->mixer_mutex);
mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
+ mutex_lock(&ctx->mixer_mutex);
+ ctx->powered = false;
+ mutex_unlock(&ctx->mixer_mutex);
+
clk_disable_unprepare(res->mixer);
if (ctx->vp_enabled) {
clk_disable_unprepare(res->vp);
@@ -1099,12 +1109,6 @@ static void mixer_poweroff(struct exynos_drm_manager
*mgr)
}
pm_runtime_put_sync(ctx->dev);
-
- mutex_lock(&ctx->mixer_mutex);
- ctx->powered = false;
-
-out:
- mutex_unlock(&ctx->mixer_mutex);
}
static void mixer_dpms(struct exynos_drm_manager *mgr, int mode)
--
1.7.9.5
[PATCH 0/5 v2] drm/exynos: fix for misc issues related to exynos mixer
Fixes for various issues which are to Power On/Off sequence, Layer update, waiting for vblank in exynos mixer driver. v2: 1) Repalce mixer_enable_vblank with drm_vblank_get. This series is based on exynos-drm-fixes branch in Inki dae's tree. Rahul Sharma (5): drm/exynos: set power state variable after enabling clocks and power drm/exynos: stop mixer before gating clocks during poweroff drm/exynos: allow mulitple layer updates per vsync for mixer drm/exynos: soft reset mixer before reconfigure after power-on drm/exynos: enable vsync interrupt while waiting for vblank drivers/gpu/drm/exynos/exynos_mixer.c | 50 +++-- drivers/gpu/drm/exynos/regs-mixer.h |1 + 2 files changed, 36 insertions(+), 15 deletions(-) -- 1.7.9.5
[PATCH 5/5] drm/exynos: enable vsync interrupt while waiting for vblank
Hi All,
On 18 June 2014 17:04, Rahul Sharma wrote:
> mixer_wait_for_vblank function expects that the upcoming
> vsync interrupt handler routine will clear the
> wait_vsync_event atomic variable.
>
> For this to happen, interrupts should be enabled and
> disabled properly.
>
> Signed-off-by: Rahul Sharma
> ---
> drivers/gpu/drm/exynos/exynos_mixer.c |4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 6f18581..7927f2e 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1019,6 +1019,8 @@ static void mixer_wait_for_vblank(struct
> exynos_drm_manager *mgr)
> }
> mutex_unlock(&mixer_ctx->mixer_mutex);
>
> + mixer_enable_vblank(mgr);
> +
> atomic_set(&mixer_ctx->wait_vsync_event, 1);
>
> /*
> @@ -1029,6 +1031,8 @@ static void mixer_wait_for_vblank(struct
> exynos_drm_manager *mgr)
> !atomic_read(&mixer_ctx->wait_vsync_event),
> HZ/20))
> DRM_DEBUG_KMS("vblank wait timed out.\n");
> +
> + mixer_disable_vblank(mgr);
> }
I found issue with this patch. It is causing deadlock by bypassing
vblank refcounting
and manually disabling the Interrupt.
I switched to following and it is back on track.
drm_vblank_get(mgr->crtc->dev, mixer_ctx->pipe);
drm_vblank_put(mgr->crtc->dev, mixer_ctx->pipe);
I will include this change in next version.
Regards,
Rahul Sharma.
>
> static void mixer_window_suspend(struct exynos_drm_manager *mgr)
> --
> 1.7.9.5
>
[RFC] drm/exynos: abort commit when framebuffer is removed from plane
This situation arises when userspace remove the frambuffer object
and call setmode ioctl.
drm_mode_rmfb --> drm_plane_force_disable --> plane->crtc = NULL;
and
drm_mode_setcrtc --> exynos_plane_commit --> passes plane->crtc to
exynos_drm_crtc_plane_commit which is NULL.
This crashes the system.
Signed-off-by: Rahul Sharma
---
This works fine but I am not confident on the correctness of the
solution.
drivers/gpu/drm/exynos/exynos_drm_crtc.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 95c9435..da4efe4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -165,6 +165,12 @@ static int exynos_drm_crtc_mode_set_commit(struct drm_crtc
*crtc, int x, int y,
return -EPERM;
}
+ /* when framebuffer is removed, commit should not proceed. */
+ if(!plane->fb){
+ DRM_ERROR("framebuffer has been removed from plane.\n");
+ return -EFAULT;
+ }
+
crtc_w = crtc->primary->fb->width - x;
crtc_h = crtc->primary->fb->height - y;
--
1.7.9.5
[PATCH] drm/exynos: defer hdmi probe when fail to get regulators
Thanks Inki,
On 19 June 2014 09:34, Inki Dae wrote:
> On 2014? 06? 18? 22:42, Rahul Sharma wrote:
>> HDMI probe proceeds with dummy regulators when the regulators
>> are not provided in DT node or regulator provider has not get
>> probed or failed to register the regulators.
>>
>> This patch modify hdmi driver to defer the probe in case the
>> regulators are not available.
>
> No, already available. See the below comments.
>
>>
>> Signed-off-by: Rahul Sharma
>> ---
>> Based on exynos-drm-fixes branch in Inki dae's tree.
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 69
>> --
>> 1 file changed, 50 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index aa259b0..3f24c49 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -83,7 +83,7 @@ struct hdmi_resources {
>> struct clk *sclk_pixel;
>> struct clk *sclk_hdmiphy;
>> struct clk *mout_hdmi;
>> - struct regulator_bulk_data *regul_bulk;
>> + struct regulator**regulators;
>> int regul_count;
>> };
>>
>> @@ -2022,6 +2022,36 @@ static void hdmi_commit(struct exynos_drm_display
>> *display)
>> hdmi_conf_apply(hdata);
>> }
>>
>> +int hdmi_regulator_enable(struct hdmi_context *hdata)
>> +{
>> + struct hdmi_resources *res = &hdata->res;
>> + int i, ret;
>> +
>> + for (i = 0; i < res->regul_count; ++i) {
>> + ret = regulator_enable(res->regulators[i]);
>> + if (ret < 0) {
>> + DRM_ERROR("fail to enable regulators.\n");
>> + return ret;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +int hdmi_regulator_disable(struct hdmi_context *hdata)
>> +{
>> + struct hdmi_resources *res = &hdata->res;
>> + int i, ret;
>> +
>> + for (i = 0; i < res->regul_count; ++i) {
>> + ret = regulator_disable(res->regulators[i]);
>> + if (ret < 0) {
>> + DRM_ERROR("fail to disable regulators.\n");
>> + return ret;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> static void hdmi_poweron(struct exynos_drm_display *display)
>> {
>> struct hdmi_context *hdata = display->ctx;
>> @@ -2039,8 +2069,8 @@ static void hdmi_poweron(struct exynos_drm_display
>> *display)
>>
>> pm_runtime_get_sync(hdata->dev);
>>
>> - if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>> - DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>> + if (hdmi_regulator_enable(hdata))
>> + DRM_DEBUG_KMS("failed to enable regulators\n");
>>
>> /* set pmu hdmiphy control bit to enable hdmiphy */
>> regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> @@ -2077,7 +2107,8 @@ static void hdmi_poweroff(struct exynos_drm_display
>> *display)
>> regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> PMU_HDMI_PHY_ENABLE_BIT, 0);
>>
>> - regulator_bulk_disable(res->regul_count, res->regul_bulk);
>> + if (hdmi_regulator_disable(hdata))
>> + DRM_DEBUG_KMS("failed to disable regulators\n");
>>
>> pm_runtime_put_sync(hdata->dev);
>>
>> @@ -2211,24 +2242,24 @@ static int hdmi_resources_init(struct hdmi_context
>> *hdata)
>>
>> clk_set_parent(res->mout_hdmi, res->sclk_pixel);
>>
>> - res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
>> - sizeof(res->regul_bulk[0]), GFP_KERNEL);
>> - if (!res->regul_bulk) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + res->regul_count = ARRAY_SIZE(supply);
>> +
>> + res->regulators = devm_kzalloc(dev, res->regul_count *
>> + sizeof(res->regulators[0]), GFP_KERNEL);
>> + if (!res->regulators)
>> + return -ENOMEM;
>> +
>> for (i = 0; i < ARRAY_SIZE(supply); ++i) {
>> - res->regul_bulk[i].supply = supply[i];
>> - res->regul_bulk[
[PATCH] drm/exynos: defer hdmi probe when fail to get regulators
HDMI probe proceeds with dummy regulators when the regulators
are not provided in DT node or regulator provider has not get
probed or failed to register the regulators.
This patch modify hdmi driver to defer the probe in case the
regulators are not available.
Signed-off-by: Rahul Sharma
---
Based on exynos-drm-fixes branch in Inki dae's tree.
drivers/gpu/drm/exynos/exynos_hdmi.c | 69 --
1 file changed, 50 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index aa259b0..3f24c49 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -83,7 +83,7 @@ struct hdmi_resources {
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
struct clk *mout_hdmi;
- struct regulator_bulk_data *regul_bulk;
+ struct regulator**regulators;
int regul_count;
};
@@ -2022,6 +2022,36 @@ static void hdmi_commit(struct exynos_drm_display
*display)
hdmi_conf_apply(hdata);
}
+int hdmi_regulator_enable(struct hdmi_context *hdata)
+{
+ struct hdmi_resources *res = &hdata->res;
+ int i, ret;
+
+ for (i = 0; i < res->regul_count; ++i) {
+ ret = regulator_enable(res->regulators[i]);
+ if (ret < 0) {
+ DRM_ERROR("fail to enable regulators.\n");
+ return ret;
+ }
+ }
+ return 0;
+}
+
+int hdmi_regulator_disable(struct hdmi_context *hdata)
+{
+ struct hdmi_resources *res = &hdata->res;
+ int i, ret;
+
+ for (i = 0; i < res->regul_count; ++i) {
+ ret = regulator_disable(res->regulators[i]);
+ if (ret < 0) {
+ DRM_ERROR("fail to disable regulators.\n");
+ return ret;
+ }
+ }
+ return 0;
+}
+
static void hdmi_poweron(struct exynos_drm_display *display)
{
struct hdmi_context *hdata = display->ctx;
@@ -2039,8 +2069,8 @@ static void hdmi_poweron(struct exynos_drm_display
*display)
pm_runtime_get_sync(hdata->dev);
- if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
- DRM_DEBUG_KMS("failed to enable regulator bulk\n");
+ if (hdmi_regulator_enable(hdata))
+ DRM_DEBUG_KMS("failed to enable regulators\n");
/* set pmu hdmiphy control bit to enable hdmiphy */
regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
@@ -2077,7 +2107,8 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
PMU_HDMI_PHY_ENABLE_BIT, 0);
- regulator_bulk_disable(res->regul_count, res->regul_bulk);
+ if (hdmi_regulator_disable(hdata))
+ DRM_DEBUG_KMS("failed to disable regulators\n");
pm_runtime_put_sync(hdata->dev);
@@ -2211,24 +2242,24 @@ static int hdmi_resources_init(struct hdmi_context
*hdata)
clk_set_parent(res->mout_hdmi, res->sclk_pixel);
- res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
- sizeof(res->regul_bulk[0]), GFP_KERNEL);
- if (!res->regul_bulk) {
- ret = -ENOMEM;
- goto fail;
- }
+ res->regul_count = ARRAY_SIZE(supply);
+
+ res->regulators = devm_kzalloc(dev, res->regul_count *
+ sizeof(res->regulators[0]), GFP_KERNEL);
+ if (!res->regulators)
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(supply); ++i) {
- res->regul_bulk[i].supply = supply[i];
- res->regul_bulk[i].consumer = NULL;
- }
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
- if (ret) {
- DRM_ERROR("failed to get regulators\n");
- return ret;
+ res->regulators[i] =
+ devm_regulator_get_optional(dev, supply[i]);
+ if (IS_ERR(res->regulators[i])) {
+ DRM_ERROR("fail to get regulator: %s.\n",
+ supply[i]);
+ return -EPROBE_DEFER;
+ }
}
- res->regul_count = ARRAY_SIZE(supply);
- return ret;
+ return 0;
fail:
DRM_ERROR("HDMI resource init - failed\n");
return ret;
--
1.7.9.5
[PATCH 5/5] drm/exynos: enable vsync interrupt while waiting for vblank
mixer_wait_for_vblank function expects that the upcoming
vsync interrupt handler routine will clear the
wait_vsync_event atomic variable.
For this to happen, interrupts should be enabled and
disabled properly.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c |4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 6f18581..7927f2e 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1019,6 +1019,8 @@ static void mixer_wait_for_vblank(struct
exynos_drm_manager *mgr)
}
mutex_unlock(&mixer_ctx->mixer_mutex);
+ mixer_enable_vblank(mgr);
+
atomic_set(&mixer_ctx->wait_vsync_event, 1);
/*
@@ -1029,6 +1031,8 @@ static void mixer_wait_for_vblank(struct
exynos_drm_manager *mgr)
!atomic_read(&mixer_ctx->wait_vsync_event),
HZ/20))
DRM_DEBUG_KMS("vblank wait timed out.\n");
+
+ mixer_disable_vblank(mgr);
}
static void mixer_window_suspend(struct exynos_drm_manager *mgr)
--
1.7.9.5
[PATCH 4/5] drm/exynos: soft reset mixer before reconfigure after power-on
Mixer soft reset is a recommended step before reconfiguring the mixer after power on. Mixer looses the previous state of DMAs if soft reset. This is the recommendation from the hardware team. Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_mixer.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 6773b03..6f18581 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1085,6 +1085,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) ctx->powered = true; mutex_unlock(&ctx->mixer_mutex); + mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET); + mixer_reg_write(res, MXR_INT_EN, ctx->int_en); mixer_win_reset(ctx); -- 1.7.9.5
[PATCH 3/5] drm/exynos: allow mulitple layer updates per vsync for mixer
Allowing only one layer update per vsync can cause issues
while there are update available for both layers. There is
a good amount of possibility to loose updates if we allow
single update per vsync.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c |7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index d359501..6773b03 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int
win)
static void mixer_layer_update(struct mixer_context *ctx)
{
struct mixer_resources *res = &ctx->mixer_res;
- u32 val;
-
- val = mixer_reg_read(res, MXR_CFG);
- /* allow one update per vsync only */
- if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
- mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
+ mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
}
static void mixer_graph_buffer(struct mixer_context *ctx, int win)
--
1.7.9.5
[PATCH 2/5] drm/exynos: stop mixer before gating clocks during poweroff
Mixer should be power gated only after it is gracefully stopped.
The recommended sequence is to Stop the mixer and wait till
it enters to IDLE state before gating the clocks and power to
the mixer.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c | 15 +++
drivers/gpu/drm/exynos/regs-mixer.h |1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index c00abbe..d359501 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -377,6 +377,20 @@ static void mixer_run(struct mixer_context *ctx)
mixer_regs_dump(ctx);
}
+static void mixer_stop(struct mixer_context *ctx)
+{
+ struct mixer_resources *res = &ctx->mixer_res;
+ int timeout = 20;
+
+ mixer_reg_writemask(res, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
+
+ while (!(mixer_reg_read(res, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
+ --timeout)
+ usleep_range(1, 12000);
+
+ mixer_regs_dump(ctx);
+}
+
static void vp_video_buffer(struct mixer_context *ctx, int win)
{
struct mixer_resources *res = &ctx->mixer_res;
@@ -1094,6 +1108,7 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr)
}
mutex_unlock(&ctx->mixer_mutex);
+ mixer_stop(ctx);
mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
diff --git a/drivers/gpu/drm/exynos/regs-mixer.h
b/drivers/gpu/drm/exynos/regs-mixer.h
index 4537026..5f32e1a 100644
--- a/drivers/gpu/drm/exynos/regs-mixer.h
+++ b/drivers/gpu/drm/exynos/regs-mixer.h
@@ -78,6 +78,7 @@
#define MXR_STATUS_BIG_ENDIAN (1 << 3)
#define MXR_STATUS_ENDIAN_MASK (1 << 3)
#define MXR_STATUS_SYNC_ENABLE (1 << 2)
+#define MXR_STATUS_REG_IDLE(1 << 1)
#define MXR_STATUS_REG_RUN (1 << 0)
/* bits for MXR_CFG */
--
1.7.9.5
[PATCH 1/5] drm/exynos: set power state variable after enabling clocks and power
Power state variable holds the state of the mixer device.
Power on and power off functions are toggling these variable
at wrong place.
State variable should be changed to true only after Runtime
PM and clocks are enabled. Else it may result to a situation
where mixer registers are accessed with device power enabled.
Similar logic for poweroff sequence.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_mixer.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4c5aed7..c00abbe 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1061,7 +1061,7 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
mutex_unlock(&ctx->mixer_mutex);
return;
}
- ctx->powered = true;
+
mutex_unlock(&ctx->mixer_mutex);
pm_runtime_get_sync(ctx->dev);
@@ -1072,6 +1072,10 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
clk_prepare_enable(res->sclk_mixer);
}
+ mutex_lock(&ctx->mixer_mutex);
+ ctx->powered = true;
+ mutex_unlock(&ctx->mixer_mutex);
+
mixer_reg_write(res, MXR_INT_EN, ctx->int_en);
mixer_win_reset(ctx);
@@ -1084,14 +1088,20 @@ static void mixer_poweroff(struct exynos_drm_manager
*mgr)
struct mixer_resources *res = &ctx->mixer_res;
mutex_lock(&ctx->mixer_mutex);
- if (!ctx->powered)
- goto out;
+ if (!ctx->powered) {
+ mutex_unlock(&ctx->mixer_mutex);
+ return;
+ }
mutex_unlock(&ctx->mixer_mutex);
mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
+ mutex_lock(&ctx->mixer_mutex);
+ ctx->powered = false;
+ mutex_unlock(&ctx->mixer_mutex);
+
clk_disable_unprepare(res->mixer);
if (ctx->vp_enabled) {
clk_disable_unprepare(res->vp);
@@ -1099,12 +1109,6 @@ static void mixer_poweroff(struct exynos_drm_manager
*mgr)
}
pm_runtime_put_sync(ctx->dev);
-
- mutex_lock(&ctx->mixer_mutex);
- ctx->powered = false;
-
-out:
- mutex_unlock(&ctx->mixer_mutex);
}
static void mixer_dpms(struct exynos_drm_manager *mgr, int mode)
--
1.7.9.5
[PATCH 0/5] drm/exynos: fix for misc issues related to exynos mixer
Fixes for various issues which are to Power On/Off sequence, Layer update, waiting for vblank in exynos mixer driver. This series is based on exynos-drm-fixes branch in Inki dae's tree. Rahul Sharma (5): drm/exynos: set power state variable after enabling clocks and power drm/exynos: stop mixer before gating clocks during poweroff drm/exynos: allow mulitple layer updates per vsync for mixer drm/exynos: soft reset mixer before reconfigure after power-on drm/exynos: enable vsync interrupt while waiting for vblank drivers/gpu/drm/exynos/exynos_mixer.c | 50 +++-- drivers/gpu/drm/exynos/regs-mixer.h |1 + 2 files changed, 36 insertions(+), 15 deletions(-) -- 1.7.9.5
[PATCH] drm/exynos: remove hardware overlays disable from fimd probe
On 2 June 2014 14:41, Andrzej Hajda wrote:
> Hi Rahul,
>
> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>> System hangs when FIMD registers are accessed to disable
>> hardware overlays. This is because of the clocks which are
>> not enabled before register access.
>>
>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>
>> Signed-off-by: Rahul Sharma
>
> This patch causes regression on some exynos4210-universal_c210 devices,
> everything works expect colors are incorrect - it seems blue component
> is very dark, almost black.
>
Oh Sorry for that. I did not see any problem on 5250/5420/5800. I do not
have setup for 4210. Better we should revert this patch.
Would you please help me by verifying the following patch on 4210? This
is an alternate solution to the same problem.
http://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg31426.html
Thanks Andrej, for bringing it to notice.
Regards,
Rahul Sharma
> Regards
> Andrzej
>
>> ---
>> Based on exynos-drm-next branch.
>>
>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 17 -
>> 1 file changed, 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bd30d0c..2ec634f 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager
>> *mgr, int zpos)
>> win_data->enabled = false;
>> }
>>
>> -static void fimd_clear_win(struct fimd_context *ctx, int win)
>> -{
>> - writel(0, ctx->regs + WINCON(win));
>> - writel(0, ctx->regs + VIDOSD_A(win));
>> - writel(0, ctx->regs + VIDOSD_B(win));
>> - writel(0, ctx->regs + VIDOSD_C(win));
>> -
>> - if (win == 1 || win == 2)
>> - writel(0, ctx->regs + VIDOSD_D(win));
>> -
>> - fimd_shadow_protect_win(ctx, win, false);
>> -}
>> -
>> static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>> {
>> struct fimd_context *ctx = mgr->ctx;
>> @@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device
>> *master, void *data)
>> {
>> struct fimd_context *ctx = fimd_manager.ctx;
>> struct drm_device *drm_dev = data;
>> - int win;
>>
>> fimd_mgr_initialize(&fimd_manager, drm_dev);
>> exynos_drm_crtc_create(&fimd_manager);
>> if (ctx->display)
>> exynos_drm_create_enc_conn(drm_dev, ctx->display);
>>
>> - for (win = 0; win < WINDOWS_NR; win++)
>> - fimd_clear_win(ctx, win);
>> -
>> return 0;
>>
>> }
>>
>
[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers
Thanks Inki,
I removed fimd_clear_win from fimd_probe and verified this change in chrome
setup. Not seeing any noticeable difference.
I have posted another patch at:
http://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg31629.html
Thanks everybody for your review effort.
Regards,
Rahul Sharma.
On 27 May 2014 18:28, Inki Dae wrote:
> On 2014? 05? 27? 18:55, Rahul Sharma wrote:
>>
>>
>> On 26 May 2014 14:21, Rahul Sharma wrote:
>>> Hi Daniel,
>>>
>>> On 26 May 2014 13:11, Daniel Kurtz wrote:
>>>> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma >>> samsung.com> wrote:
>>>>>
>>>>> Hi Inki,
>>>>>
>>>>> Please review this patch.
>>> [snip]
>>>>>> +
>>>>>> + ret = clk_prepare_enable(ctx->lcd_clk);
>>>>
>>>> Hi Rahul,
>>>>
>>>> Can you explain why exactly we are "clearing windows" here in probe(),
>>>> anyway?
>>>
>>> I am not sure why it was added there but it is present since the first
>>> version of this
>>> file. Probably Inki can explain about this :). I can see the change
>>> coming from his
>>> first patch for adding drm fimd driver.
>>>
>>>>
>>>> IIUC, bus_clk is the clock that enables FIMD register access, and
>>>> lcd_clk clocks the scan out engine.
>>>> Therefore, if we only need to read/write some registers, we only need
>>>> the bus_clk, not lcd_clk, right?
>>>>
>>>
>>> Correct, bus_clk should be sufficient to access the registers. But unless we
>>> are confident about all implicit clock requirements in all SoCs, it is
>>> safer to follow
>>> the power_on/off sequence. This implementation is as good as DPMS on ->
>>> perform
>>> reg operation -> DPMS Off. It was same in the original version but
>>> later clock enables
>>> were moved out of the probe.
>>>
>>>> However, fimd_clear_win() actually clears per-window registers.
>>>> Writes to per-window registers typically do not take effect until the
>>>> next vblank.
>>>> Therefore we do would need to enable lcd_clk to ensure that these
>>>> changes take effect.
>>>> Furthermore, to ensure the window clear completes during probe(), we
>>>> would also need to synchronously wait for the next vblank here - but
>>>> only if FIMD scanout is actually enabled already, otherwise there will
>>>> never be a next scanout, so we must check for that first.
>>>> Lastly, waiting around for a vblank could take a while. Doing so in
>>>> probe() is not very friendly to boot up time, so the waiting should
>>>> probably be moved out of the main probe() thread into some sort of
>>>> asynchronous handler, which could then signal back when the clear is
>>>> complete.
>>>>
>>>> Do you agree, or am I missing something?
>>>
>>> I agree. There seems a room for improvement. But at present we have two
>>> options,
>>> either fix the current implementation and try to improve it as you mentioned
>>> above. OR remove fimd_clear_win from probe if it is just a legacy code which
>
> Just let's remove fimd_clear_win. it wouldn't need to disable all
> hardware overlays at probe.
>
> Thanks,
> Inki Dae
>
>>> is no more required.
>>>
>>> @Inki, need your inputs here.
>>>
>>> Regards,
>>> Rahul Sharma.
>>>
>>>>
>>>> Thanks,
>>>> -djk
>>>>
>>>>>
>>>>>> + if (ret) {
>>> [snip]
>>>>>> +pm_put_err:
>>>>>> + return ret;
>>>>>> }
>>>>>>
>>>>>> static void fimd_unbind(struct device *dev, struct device *master,
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>> ___
>>>>> dri-devel mailing list
>>>>> dri-devel at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>> ___
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: remove hardware overlays disable from fimd probe
System hangs when FIMD registers are accessed to disable
hardware overlays. This is because of the clocks which are
not enabled before register access.
'Hardware overlay disable' is cleaned from the FIMD probe.
Signed-off-by: Rahul Sharma
---
Based on exynos-drm-next branch.
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..2ec634f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager
*mgr, int zpos)
win_data->enabled = false;
}
-static void fimd_clear_win(struct fimd_context *ctx, int win)
-{
- writel(0, ctx->regs + WINCON(win));
- writel(0, ctx->regs + VIDOSD_A(win));
- writel(0, ctx->regs + VIDOSD_B(win));
- writel(0, ctx->regs + VIDOSD_C(win));
-
- if (win == 1 || win == 2)
- writel(0, ctx->regs + VIDOSD_D(win));
-
- fimd_shadow_protect_win(ctx, win, false);
-}
-
static void fimd_window_suspend(struct exynos_drm_manager *mgr)
{
struct fimd_context *ctx = mgr->ctx;
@@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device
*master, void *data)
{
struct fimd_context *ctx = fimd_manager.ctx;
struct drm_device *drm_dev = data;
- int win;
fimd_mgr_initialize(&fimd_manager, drm_dev);
exynos_drm_crtc_create(&fimd_manager);
if (ctx->display)
exynos_drm_create_enc_conn(drm_dev, ctx->display);
- for (win = 0; win < WINDOWS_NR; win++)
- fimd_clear_win(ctx, win);
-
return 0;
}
--
1.7.9.5
[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers
On 26 May 2014 14:21, Rahul Sharma wrote:
> Hi Daniel,
>
> On 26 May 2014 13:11, Daniel Kurtz wrote:
>> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma
>> wrote:
>>>
>>> Hi Inki,
>>>
>>> Please review this patch.
> [snip]
>>> > +
>>> > + ret = clk_prepare_enable(ctx->lcd_clk);
>>
>> Hi Rahul,
>>
>> Can you explain why exactly we are "clearing windows" here in probe(),
>> anyway?
>
> I am not sure why it was added there but it is present since the first
> version of this
> file. Probably Inki can explain about this :). I can see the change
> coming from his
> first patch for adding drm fimd driver.
>
>>
>> IIUC, bus_clk is the clock that enables FIMD register access, and
>> lcd_clk clocks the scan out engine.
>> Therefore, if we only need to read/write some registers, we only need
>> the bus_clk, not lcd_clk, right?
>>
>
> Correct, bus_clk should be sufficient to access the registers. But unless we
> are confident about all implicit clock requirements in all SoCs, it is
> safer to follow
> the power_on/off sequence. This implementation is as good as DPMS on ->
> perform
> reg operation -> DPMS Off. It was same in the original version but
> later clock enables
> were moved out of the probe.
>
>> However, fimd_clear_win() actually clears per-window registers.
>> Writes to per-window registers typically do not take effect until the
>> next vblank.
>> Therefore we do would need to enable lcd_clk to ensure that these
>> changes take effect.
>> Furthermore, to ensure the window clear completes during probe(), we
>> would also need to synchronously wait for the next vblank here - but
>> only if FIMD scanout is actually enabled already, otherwise there will
>> never be a next scanout, so we must check for that first.
>> Lastly, waiting around for a vblank could take a while. Doing so in
>> probe() is not very friendly to boot up time, so the waiting should
>> probably be moved out of the main probe() thread into some sort of
>> asynchronous handler, which could then signal back when the clear is
>> complete.
>>
>> Do you agree, or am I missing something?
>
> I agree. There seems a room for improvement. But at present we have two
> options,
> either fix the current implementation and try to improve it as you mentioned
> above. OR remove fimd_clear_win from probe if it is just a legacy code which
> is no more required.
>
> @Inki, need your inputs here.
>
> Regards,
> Rahul Sharma.
>
>>
>> Thanks,
>> -djk
>>
>>>
>>> > + if (ret) {
> [snip]
>>> > +pm_put_err:
>>> > + return ret;
>>> > }
>>> >
>>> > static void fimd_unbind(struct device *dev, struct device *master,
>>> > --
>>> > 1.7.9.5
>>> >
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers
Hi Daniel,
On 26 May 2014 13:11, Daniel Kurtz wrote:
> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma
> wrote:
>>
>> Hi Inki,
>>
>> Please review this patch.
[snip]
>> > +
>> > + ret = clk_prepare_enable(ctx->lcd_clk);
>
> Hi Rahul,
>
> Can you explain why exactly we are "clearing windows" here in probe(), anyway?
I am not sure why it was added there but it is present since the first
version of this
file. Probably Inki can explain about this :). I can see the change
coming from his
first patch for adding drm fimd driver.
>
> IIUC, bus_clk is the clock that enables FIMD register access, and
> lcd_clk clocks the scan out engine.
> Therefore, if we only need to read/write some registers, we only need
> the bus_clk, not lcd_clk, right?
>
Correct, bus_clk should be sufficient to access the registers. But unless we
are confident about all implicit clock requirements in all SoCs, it is
safer to follow
the power_on/off sequence. This implementation is as good as DPMS on -> perform
reg operation -> DPMS Off. It was same in the original version but
later clock enables
were moved out of the probe.
> However, fimd_clear_win() actually clears per-window registers.
> Writes to per-window registers typically do not take effect until the
> next vblank.
> Therefore we do would need to enable lcd_clk to ensure that these
> changes take effect.
> Furthermore, to ensure the window clear completes during probe(), we
> would also need to synchronously wait for the next vblank here - but
> only if FIMD scanout is actually enabled already, otherwise there will
> never be a next scanout, so we must check for that first.
> Lastly, waiting around for a vblank could take a while. Doing so in
> probe() is not very friendly to boot up time, so the waiting should
> probably be moved out of the main probe() thread into some sort of
> asynchronous handler, which could then signal back when the clear is
> complete.
>
> Do you agree, or am I missing something?
I agree. There seems a room for improvement. But at present we have two options,
either fix the current implementation and try to improve it as you mentioned
above. OR remove fimd_clear_win from probe if it is just a legacy code which
is no more required.
@Inki, need your inputs here.
Regards,
Rahul Sharma.
>
> Thanks,
> -djk
>
>>
>> > + if (ret) {
[snip]
>> > +pm_put_err:
>> > + return ret;
>> > }
>> >
>> > static void fimd_unbind(struct device *dev, struct device *master,
>> > --
>> > 1.7.9.5
>> >
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers
Hi Inki,
Please review this patch.
Regards,
Rahul Sharma
On 23 May 2014 17:17, Rahul Sharma wrote:
> From: Rahul Sharma
>
> Fimd probe is accessing fimd Registers without enabling the fimd
> gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled
> during kernel boottime, the system hangs during boottime.
>
> This issue got surfaced when verifying with sysmmu enabled. Probe of
> fimd Sysmmu enables the master clock before accessing sysmmu regs and
> then disables. Later fimd probe tries to read the register without
> enabling the clock which is wrong and hangs the system.
>
> Signed-off-by: Rahul Sharma
> ---
> v5:
> 1) Added pm_runtime_get_sync to enable the Display power domain.
> v4:
> 1) Added clk_disable for prev clock when clk_enable fails.
> v3:
> 1) Added checks for clk_enable.
> v2:
> Rebase.
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 35
> +-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bd30d0c..6a30415 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -898,18 +898,51 @@ static int fimd_bind(struct device *dev, struct device
> *master, void *data)
> {
> struct fimd_context *ctx = fimd_manager.ctx;
> struct drm_device *drm_dev = data;
> - int win;
> + int win, ret;
>
> fimd_mgr_initialize(&fimd_manager, drm_dev);
> exynos_drm_crtc_create(&fimd_manager);
> if (ctx->display)
> exynos_drm_create_enc_conn(drm_dev, ctx->display);
>
> + ret = pm_runtime_get_sync(ctx->dev);
> + if (ret) {
> + dev_err(dev, "pm runtime get has failed.\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(ctx->bus_clk);
> + if (ret) {
> + dev_err(dev, "bus clock enable failed.\n");
> + goto bus_clk_err;
> + }
> +
> + ret = clk_prepare_enable(ctx->lcd_clk);
> + if (ret) {
> + dev_err(dev, "lcd clock enable failed.\n");
> + goto lcd_clk_err;
> + }
> +
> for (win = 0; win < WINDOWS_NR; win++)
> fimd_clear_win(ctx, win);
>
> + clk_disable_unprepare(ctx->lcd_clk);
> + clk_disable_unprepare(ctx->bus_clk);
> +
> + ret = pm_runtime_put_sync(ctx->dev);
> + if (ret) {
> + dev_err(dev, "pm runtime put has failed.\n");
> + goto pm_put_err;
> + }
> +
> return 0;
>
> +lcd_clk_err:
> + clk_disable_unprepare(ctx->bus_clk);
> +bus_clk_err:
> + pm_runtime_put_sync(ctx->dev);
> +pm_put_err:
> + return ret;
> }
>
> static void fimd_unbind(struct device *dev, struct device *master,
> --
> 1.7.9.5
>
[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers
From: Rahul Sharma
Fimd probe is accessing fimd Registers without enabling the fimd
gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled
during kernel boottime, the system hangs during boottime.
This issue got surfaced when verifying with sysmmu enabled. Probe of
fimd Sysmmu enables the master clock before accessing sysmmu regs and
then disables. Later fimd probe tries to read the register without
enabling the clock which is wrong and hangs the system.
Signed-off-by: Rahul Sharma
---
v5:
1) Added pm_runtime_get_sync to enable the Display power domain.
v4:
1) Added clk_disable for prev clock when clk_enable fails.
v3:
1) Added checks for clk_enable.
v2:
Rebase.
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 35 +-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..6a30415 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -898,18 +898,51 @@ static int fimd_bind(struct device *dev, struct device
*master, void *data)
{
struct fimd_context *ctx = fimd_manager.ctx;
struct drm_device *drm_dev = data;
- int win;
+ int win, ret;
fimd_mgr_initialize(&fimd_manager, drm_dev);
exynos_drm_crtc_create(&fimd_manager);
if (ctx->display)
exynos_drm_create_enc_conn(drm_dev, ctx->display);
+ ret = pm_runtime_get_sync(ctx->dev);
+ if (ret) {
+ dev_err(dev, "pm runtime get has failed.\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(ctx->bus_clk);
+ if (ret) {
+ dev_err(dev, "bus clock enable failed.\n");
+ goto bus_clk_err;
+ }
+
+ ret = clk_prepare_enable(ctx->lcd_clk);
+ if (ret) {
+ dev_err(dev, "lcd clock enable failed.\n");
+ goto lcd_clk_err;
+ }
+
for (win = 0; win < WINDOWS_NR; win++)
fimd_clear_win(ctx, win);
+ clk_disable_unprepare(ctx->lcd_clk);
+ clk_disable_unprepare(ctx->bus_clk);
+
+ ret = pm_runtime_put_sync(ctx->dev);
+ if (ret) {
+ dev_err(dev, "pm runtime put has failed.\n");
+ goto pm_put_err;
+ }
+
return 0;
+lcd_clk_err:
+ clk_disable_unprepare(ctx->bus_clk);
+bus_clk_err:
+ pm_runtime_put_sync(ctx->dev);
+pm_put_err:
+ return ret;
}
static void fimd_unbind(struct device *dev, struct device *master,
--
1.7.9.5
[PATCH v4] drm/exynos: enable fimd clocks in probe before accessing fimd registers
Hi Andrej, On 23 May 2014 13:13, Andrzej Hajda wrote: > Hi Rahul, > > [snip] >> + clk_disable_unprepare(ctx->lcd_clk); >> + clk_disable_unprepare(ctx->bus_clk); >> + >> return 0; > > If you want to access fimd registers I guess pm_runtime_get_sync should > be called as well, to wake up display pm domain. > You are right. I have added pm runtime get sync and put sync. Regards, Rahul Sharma. > > Regards > Andrzej > > >> >> } >> >
[PATCH v4] drm/exynos: enable fimd clocks in probe before accessing fimd registers
From: Rahul Sharma
Fimd probe is accessing fimd Registers without enabling the fimd
gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled
during kernel boottime, the system hangs during boottime.
This issue got surfaced when verifying with sysmmu enabled. Probe of
fimd Sysmmu enables the master clock before accessing sysmmu regs and
then disables. Later fimd probe tries to read the register without
enabling the clock which is wrong and hangs the system.
Signed-off-by: Rahul Sharma
---
v4:
1) Added clk_disable for prev clock when clk_enable fails.
v3:
1) Added checks for clk_enable.
v2:
Rebase.
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..30ccd67 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -898,16 +898,32 @@ static int fimd_bind(struct device *dev, struct device
*master, void *data)
{
struct fimd_context *ctx = fimd_manager.ctx;
struct drm_device *drm_dev = data;
- int win;
+ int win, ret;
fimd_mgr_initialize(&fimd_manager, drm_dev);
exynos_drm_crtc_create(&fimd_manager);
if (ctx->display)
exynos_drm_create_enc_conn(drm_dev, ctx->display);
+ ret = clk_prepare_enable(ctx->bus_clk);
+ if (ret) {
+ dev_err(dev, "bus clock enable failed.\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(ctx->lcd_clk);
+ if (ret) {
+ dev_err(dev, "lcd clock enable failed.\n");
+ clk_disable_unprepare(ctx->bus_clk);
+ return ret;
+ }
+
for (win = 0; win < WINDOWS_NR; win++)
fimd_clear_win(ctx, win);
+ clk_disable_unprepare(ctx->lcd_clk);
+ clk_disable_unprepare(ctx->bus_clk);
+
return 0;
}
--
1.7.9.5
[PATCH] drm/exynos: fix nested calls to lock mutex in drm resume
On 22 May 2014 23:26, Sachin Kamat wrote: > Hi Rahul, > > On 22 May 2014 19:46, Rahul Sharma wrote: >> Hi Inki, >> >> This is another one which has not got reviewed. Please review. > > Inki has applied a similar patch from Takashi [1]. Thanks Sachin, Good that solution is merged. @ Inki, sorry for raising concern. It went unnoticed. Regards, Rahul Sharma. > > [1] https://lkml.org/lkml/2014/5/9/24 > > -- > With warm regards, > Sachin
[PATCH] drm/exynos: fix nested calls to lock mutex in drm resume
Hi Inki, This is another one which has not got reviewed. Please review. Regards, Rahul Sharma On 30 April 2014 19:11, Rahul Sharma wrote: > From: Rahul Sharma > > While testing S2R on exynos board, system is hanging and > rebooting due to nested mutex_lock calls in exynos drm > resume callback. Changing the order of the calls is fixing > the issue. > > Signed-off-by: Rahul Sharma > --- > Based on exynos-drm-next branch in Inki Dae's tree. > drivers/gpu/drm/exynos/exynos_drm_drv.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index bb7dfee..2bb6233 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -184,8 +184,8 @@ static int exynos_drm_resume(struct drm_device *dev) > connector->funcs->dpms(connector, connector->dpms); > } > > - drm_helper_resume_force_mode(dev); > drm_modeset_unlock_all(dev); > + drm_helper_resume_force_mode(dev); > > return 0; > } > -- > 1.7.9.5 >
[PATCH 5/7] drm/exynos: add hdmiphy power on/off sequence
Hi Inki,
Please review this patch.
Regards,
Rahul Sharma
On 3 April 2014 20:41, Rahul Sharma wrote:
> From: Shirish S
>
> This patch implements the power on/off sequence
> of HDMI PHY in exynos5420 and exynos5250 as provided
> by the hardware team.
>
> This has been verified for mulitple iterations of
> S2R.
>
> Signed-off-by: Shirish S
> Signed-off-by: Rahul Sharma
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 40
> +-
> drivers/gpu/drm/exynos/regs-hdmi.h |7 +-
> 2 files changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 539e603..b2cbf43 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1711,16 +1711,44 @@ static void hdmiphy_conf_reset(struct hdmi_context
> *hdata)
>
> static void hdmiphy_poweron(struct hdmi_context *hdata)
> {
> - if (hdata->type == HDMI_TYPE14)
> - hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0,
> - HDMI_PHY_POWER_OFF_EN);
> + if (hdata->type != HDMI_TYPE14)
> + return;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* For PHY Mode Setting */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
> + HDMI_PHY_ENABLE_MODE_SET);
> + /* Phy Power On */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
> + HDMI_PHY_POWER_ON);
> + /* For PHY Mode Setting */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
> + HDMI_PHY_DISABLE_MODE_SET);
> + /* PHY SW Reset */
> + hdmiphy_conf_reset(hdata);
> }
>
> static void hdmiphy_poweroff(struct hdmi_context *hdata)
> {
> - if (hdata->type == HDMI_TYPE14)
> - hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0,
> - HDMI_PHY_POWER_OFF_EN);
> + if (hdata->type != HDMI_TYPE14)
> + return;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* PHY SW Reset */
> + hdmiphy_conf_reset(hdata);
> + /* For PHY Mode Setting */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
> + HDMI_PHY_ENABLE_MODE_SET);
> +
> + /* PHY Power Off */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
> + HDMI_PHY_POWER_OFF);
> +
> + /* For PHY Mode Setting */
> + hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
> + HDMI_PHY_DISABLE_MODE_SET);
> }
>
> static void hdmiphy_conf_apply(struct hdmi_context *hdata)
> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h
> b/drivers/gpu/drm/exynos/regs-hdmi.h
> index 344a5db..fd4c590 100644
> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
> @@ -579,7 +579,12 @@
> #define HDMI_TG_3D HDMI_TG_BASE(0x00F0)
>
> /* HDMI PHY Registers Offsets*/
> -#define HDMIPHY_MODE_SET_DONE (0x7C >> 2)
> +#define HDMIPHY_POWER (0x74 >> 2)
> +#define HDMIPHY_MODE_SET_DONE (0x7c >> 2)
> +
> +/* HDMI PHY Values */
> +#define HDMI_PHY_POWER_ON 0x80
> +#define HDMI_PHY_POWER_OFF 0xff
>
> /* HDMI PHY Values */
> #define HDMI_PHY_DISABLE_MODE_SET 0x80
> --
> 1.7.9.5
>
[PATCH v3] drm/exynos: enable fimd clocks in probe before accessing fimd registers
Fimd probe is accessing fimd Registers without enabling the fimd
gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled
during kernel boottime, the system hangs during boottime.
This issue got surfaced when verifying with sysmmu enabled. Probe of
fimd Sysmmu enables the master clock before accessing sysmmu regs and
then disables. Later fimd probe tries to read the register without
enabling the clock which is wrong and hangs the system.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..38b77ed 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -898,16 +898,31 @@ static int fimd_bind(struct device *dev, struct device
*master, void *data)
{
struct fimd_context *ctx = fimd_manager.ctx;
struct drm_device *drm_dev = data;
- int win;
+ int win, ret;
fimd_mgr_initialize(&fimd_manager, drm_dev);
exynos_drm_crtc_create(&fimd_manager);
if (ctx->display)
exynos_drm_create_enc_conn(drm_dev, ctx->display);
+ ret = clk_prepare_enable(ctx->bus_clk);
+ if (ret) {
+ dev_err(dev, "bus clock enable failed.\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(ctx->lcd_clk);
+ if (ret) {
+ dev_err(dev, "lcd clock enable failed.\n");
+ return ret;
+ }
+
for (win = 0; win < WINDOWS_NR; win++)
fimd_clear_win(ctx, win);
+ clk_disable_unprepare(ctx->lcd_clk);
+ clk_disable_unprepare(ctx->bus_clk);
+
return 0;
}
--
1.7.9.5
[PATCH V2] drm/exynos: enable fimd clocks in probe before accessing fimd registers
On 22 May 2014 13:33, Thierry Reding wrote: > On Thu, May 22, 2014 at 12:06:16PM +0530, Rahul Sharma wrote: >> On 22 May 2014 11:51, Sachin Kamat wrote: >> > Hi Rahul, >> [snip] >> >> >> >> + clk_prepare_enable(ctx->bus_clk); >> > >> > Probably a check for its success? >> > >> >> + clk_prepare_enable(ctx->lcd_clk); >> > >> >> Generally we don't check this in any of the driver. It will be >> quite unnecessary. > > Then those drivers are all buggy. There's a reason why this function > returns an int rather than void. Just because you've never seen it fail > doesn't mean it can't. Okay... I don't mind putting extra checks. V3 is coming :). Best Regards, Rahul Sharma > > Thierry > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH] drm/exynos: use 4WORD dma burst length for small fbs
On 22 May 2014 12:25, Inki Dae wrote:
> On 2014? 05? 22? 13:36, Rahul Sharma wrote:
>> Hi Inki,
>>
>> On 21 May 2014 16:43, Inki Dae wrote:
>>>
>>> Hi Rahul,
>>>
>>> On 2014? 05? 07? 20:25, Rahul Sharma wrote:
>>>> From: Rahul Sharma
>>>>
>>>> In case of exynos, setting dma-burst to 16Word causes permanent
>>>> tearing for very small buffers, e.g. cursor buffer. Burst Mode
>>>> switching, which is based on overlay size is not recommended as
>>>> overlay size varies a lot towards the end of the screen. This
>>>> causes unstable DMA which results into tearing again.
>> [snip]
>>>> + /*
>>>> + * In case of exynos, setting dma-burst to 16Word causes permanent
>>>> + * tearing for very small buffers, e.g. cursor buffer. Burst Mode
>>>> + * switching which is based on overlay size is not recommended as
>>>> + * overlay size varies alot towards the end of the screen and rapid
>>>> + * movement causes unstable DMA which results into iommu crash/tear.
>>>
>>> FIMD has width limitation so width of hardware overlay may need to be
>>> aligned to 16 pixels.
>>> We had faced with similar issue and the issue had been resolved by
>>> aligning it to 16 pixels.
>>>
>>> So can you try to align it instead of changing burst len size?
>>>
>>
>> This problem is only with the very small FBs which are
>> rendered towards corners. For large FBs like 1366x768,
>> we dont see this issue though 1366 is not 16 pixel
>> aligned.
>
> Right, we had test it with 8x64 pixels. the limitation would be decided
> by memory bus length(ML), DMA burst length(DL) and pixel size in bytes(PS).
> And that can be calculated like below, which was guided by hardware team,
>
> Align { (ML * DI / PS) + (4bytes / PS), 2}
What will be the value for ML in Exynos? 4, I guess ?
>
> It seems too big value. Actually, 16 pixels works well and I don't see
> why it works well although out of limitation.
Actually Peach Pit and Peach Pi, native LCD resolution is 1366x768
which has no issues.
In above experiments, I rendered FB of size 96*64 which corrupts when
overlay width reduces beyond 48 Pixels for 5420, 5800 and 64 for 5250.
Probably we have not tested rendering the Small buffers (<96 Pixels) with
16 bit DMA burst and towards end of FB. As if now, this scenario only
applicable for Chrome which is having cursor.
Please let me know how to proceed further.
Regards,
Rahul Sharma.
>
> Thanks,
> Inki Dae
>
>>
>> But I still carried out following experiments just to
>> verify the fimd behaviour for small overlay.
>>
>> 1) Round DOWN:
>> + overlay->crtc_width = ((overlay->crtc_width)/16)*16;
>>
>> Cursor corruption from A to B
>> A: [ 308.53] start addr = 0x20408000, end addr = 0x2040e000, size =
>> 0x6000
>> [ 308.53] ovl_width = 64, ovl_height = 64
>> [ 308.54] offset_x = 1302, offset_y = 335
>> [ 308.54] fb_width = 96, fb_height = 64
>>
>> B: [ 341.89] start addr = 0x20408000, end addr = 0x2040e000, size =
>> 0x6000
>> [ 341.89] ovl_width = 48, ovl_height = 64
>> [ 341.90] offset_x = 1303, offset_y = 335
>> [ 341.90] fb_width = 96, fb_height = 64
>>
>>
>> 2) Round UP:
>>
>> + overlay->crtc_width = ((overlay->crtc_width + 16)/16)*16;
>> + if(overlay->crtc_width > overlay->fb_width)
>> + overlay->crtc_width = overlay->fb_width;
>>
>> Cursor corruption from A to B
>> A: [ 63.20] start addr = 0x20408000, end addr = 0x2040e000, size =
>> 0x6000
>> [ 63.21] ovl_width = 96, ovl_height = 64
>> [ 63.21] offset_x = 1271, offset_y = 346
>> [ 63.22] fb_width = 96, fb_height = 64
>>
>> B: [ 68.60] start addr = 0x20408000, end addr = 0x2040e000, size =
>> 0x6000
>> [ 68.61] ovl_width = 96, ovl_height = 64
>> [ 68.61] offset_x = 1272, offset_y = 346
>> [ 68.62] fb_width = 96, fb_height = 64
>>
>> In both the scenarios ovl_width is always 16 Pixel aligned.
>> Please let me know if you have something else in mind. I
>> will check and share the observations.
>>
>> The solution in the patch is proposed by hardware team
>> and work fine for exynos 5250, 5420 and 5800.
>>
>> Regards,
>> Rahul Sharma
>>
>>> Thanks,
>>> Inki Dae
>>>
>>>> + */
>>>> +
>>>> + if (win_data->fb_width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
>>>> + val &= ~WINCONx_BURSTLEN_MASK;
>>>> + val |= WINCONx_BURSTLEN_4WORD;
>>>> + }
>>>> +
>>>> writel(val, ctx->regs + WINCON(win));
>>>> }
>>>>
>>>>
>>>
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V2] drm/exynos: enable fimd clocks in probe before accessing fimd registers
On 22 May 2014 11:51, Sachin Kamat wrote: > Hi Rahul, [snip] >> >> + clk_prepare_enable(ctx->bus_clk); > > Probably a check for its success? > >> + clk_prepare_enable(ctx->lcd_clk); > Generally we don't check this in any of the driver. It will be quite unnecessary. Regards, Rahul Sharma > ditto. > > -- > With warm regards, > Sachin
[PATCH] drm/exynos: allocate non-contigous buffers when iommu is enabled
Thanks Sachin.
On 22 May 2014 11:38, Sachin Kamat wrote:
> Hi Rahul,
>
> On 7 May 2014 17:21, Rahul Sharma wrote:
>> From: Rahul Sharma
>>
>> Allow to allocate non-contigous buffers when iommu is enabled.
>> Currently, it tries to allocates contigous buffer which consistently
>> fail for large buffers and then fall back to non contigous. Apart
>> from being slow, this implementation is also very noisy and fills
>> the screen with alloc fail logs.
>>
>> Change-Id: I523e95aa308122ed2edc55e065ae6eb8be996541
>
> Not needed.
>
>> Signed-off-by: Rahul Sharma
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 22 ++
>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 5d88924..7136945 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -624,22 +624,20 @@ int exynos_drm_gem_dumb_create(struct drm_file
>> *file_priv,
>> args->pitch = args->width * ((args->bpp + 7) / 8);
>> args->size = args->pitch * args->height;
>>
>> - exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG |
>> - EXYNOS_BO_WC, args->size);
>> - /*
>> -* If physically contiguous memory allocation fails and if IOMMU is
>> -* supported then try to get buffer from non physically contiguous
>> -* memory area.
>> -*/
>
> This comment could be retained after suitable modification to reflect
> current logic.
>
ok. Posting V2.
Regards,
Rahul Sharma
>> - if (IS_ERR(exynos_gem_obj) && is_drm_iommu_supported(dev)) {
>> - dev_warn(dev->dev, "contiguous FB allocation failed, falling
>> back to non-contiguous\n");
>> + if (is_drm_iommu_supported(dev)) {
>> + exynos_gem_obj = exynos_drm_gem_create(dev,
>> + EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
>> + args->size);
>> + } else {
>> exynos_gem_obj = exynos_drm_gem_create(dev,
>> - EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
>> - args->size);
>> + EXYNOS_BO_CONTIG | EXYNOS_BO_WC,
>> + args->size);
>> }
>>
>> - if (IS_ERR(exynos_gem_obj))
>> + if (IS_ERR(exynos_gem_obj)) {
>> + dev_warn(dev->dev, "FB allocation failed.\n");
>> return PTR_ERR(exynos_gem_obj);
>> + }
>>
>> ret = exynos_drm_gem_handle_create(&exynos_gem_obj->base, file_priv,
>> &args->handle);
>> --
>> 1.7.9.5
>
> Otherwise looks good.
> Reviewed-by: Sachin Kamat
>
> --
> With warm regards,
> Sachin
[PATCH v2] drm:exynos: allocate non-contigous buffer when iommu is enabled
From: Rahul Sharma
Allow to allocate non-contigous buffers when iommu is enabled.
Currently, it tries to allocates contigous buffer which consistently
fail for large buffers and then fall back to non contigous. Apart
from being slow, this implementation is also very noisy and fills
the screen with alloc fail logs.
Signed-off-by: Rahul Sharma
Reviewed-by: Sachin Kamat
---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 42d2904..f323b32 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -612,22 +612,25 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
args->pitch = args->width * ((args->bpp + 7) / 8);
args->size = args->pitch * args->height;
- exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG |
- EXYNOS_BO_WC, args->size);
/*
-* If physically contiguous memory allocation fails and if IOMMU is
-* supported then try to get buffer from non physically contiguous
-* memory area.
-*/
- if (IS_ERR(exynos_gem_obj) && is_drm_iommu_supported(dev)) {
- dev_warn(dev->dev, "contiguous FB allocation failed, falling
back to non-contiguous\n");
+ * If iommu is supported, allocate non-contigous buffer else
+ * allocate physically contiguous memory.
+ */
+
+ if (is_drm_iommu_supported(dev)) {
+ exynos_gem_obj = exynos_drm_gem_create(dev,
+ EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
+ args->size);
+ } else {
exynos_gem_obj = exynos_drm_gem_create(dev,
- EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
- args->size);
+ EXYNOS_BO_CONTIG | EXYNOS_BO_WC,
+ args->size);
}
- if (IS_ERR(exynos_gem_obj))
+ if (IS_ERR(exynos_gem_obj)) {
+ dev_warn(dev->dev, "FB allocation failed.\n");
return PTR_ERR(exynos_gem_obj);
+ }
ret = exynos_drm_gem_handle_create(&exynos_gem_obj->base, file_priv,
&args->handle);
--
1.7.9.5
[PATCH] drm/exynos: allocate non-contigous buffers when iommu is enabled
Hi Inki,
The below fix doesn't affect the FB dev buffer allocation. IMO the scenario
where u-boot allocates the buffer is not disturbed.
Please review the implementation.
Regards,
Rahul Sharma.
On 7 May 2014 17:21, Rahul Sharma wrote:
> From: Rahul Sharma
>
> Allow to allocate non-contigous buffers when iommu is enabled.
> Currently, it tries to allocates contigous buffer which consistently
> fail for large buffers and then fall back to non contigous. Apart
> from being slow, this implementation is also very noisy and fills
> the screen with alloc fail logs.
>
> Change-Id: I523e95aa308122ed2edc55e065ae6eb8be996541
> Signed-off-by: Rahul Sharma
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 5d88924..7136945 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -624,22 +624,20 @@ int exynos_drm_gem_dumb_create(struct drm_file
> *file_priv,
> args->pitch = args->width * ((args->bpp + 7) / 8);
> args->size = args->pitch * args->height;
>
> - exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG |
> - EXYNOS_BO_WC, args->size);
> - /*
> -* If physically contiguous memory allocation fails and if IOMMU is
> -* supported then try to get buffer from non physically contiguous
> -* memory area.
> -*/
> - if (IS_ERR(exynos_gem_obj) && is_drm_iommu_supported(dev)) {
> - dev_warn(dev->dev, "contiguous FB allocation failed, falling
> back to non-contiguous\n");
> + if (is_drm_iommu_supported(dev)) {
> + exynos_gem_obj = exynos_drm_gem_create(dev,
> + EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
> + args->size);
> + } else {
> exynos_gem_obj = exynos_drm_gem_create(dev,
> - EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
> - args->size);
> + EXYNOS_BO_CONTIG | EXYNOS_BO_WC,
> + args->size);
> }
>
> - if (IS_ERR(exynos_gem_obj))
> + if (IS_ERR(exynos_gem_obj)) {
> + dev_warn(dev->dev, "FB allocation failed.\n");
> return PTR_ERR(exynos_gem_obj);
> + }
>
> ret = exynos_drm_gem_handle_create(&exynos_gem_obj->base, file_priv,
> &args->handle);
> --
> 1.7.9.5
>
[PATCH V2] drm/exynos: enable fimd clocks in probe before accessing fimd registers
From: Rahul Sharma Fimd probe is accessing fimd Registers without enabling the fimd gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled during kernel boottime, the system hangs during boottime. This issue got surfaced when verifying with sysmmu enabled. Probe of fimd Sysmmu enables the master clock before accessing sysmmu regs and then disables. Later fimd probe tries to read the register without enabling the clock which is wrong and hangs the system. Signed-off-by: Rahul Sharma --- Rebased on exynos-drm-next branch. drivers/gpu/drm/exynos/exynos_drm_fimd.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 173ee97..a79ba0a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -891,9 +891,15 @@ static int fimd_bind(struct device *dev, struct device *master, void *data) if (ctx->display) exynos_drm_create_enc_conn(drm_dev, ctx->display); + clk_prepare_enable(ctx->bus_clk); + clk_prepare_enable(ctx->lcd_clk); + for (win = 0; win < WINDOWS_NR; win++) fimd_clear_win(ctx, win); + clk_disable_unprepare(ctx->lcd_clk); + clk_disable_unprepare(ctx->bus_clk); + return 0; } -- 1.7.9.5
[PATCH] drm/exynos: use 4WORD dma burst length for small fbs
Hi Inki,
On 21 May 2014 16:43, Inki Dae wrote:
>
> Hi Rahul,
>
> On 2014? 05? 07? 20:25, Rahul Sharma wrote:
>> From: Rahul Sharma
>>
>> In case of exynos, setting dma-burst to 16Word causes permanent
>> tearing for very small buffers, e.g. cursor buffer. Burst Mode
>> switching, which is based on overlay size is not recommended as
>> overlay size varies a lot towards the end of the screen. This
>> causes unstable DMA which results into tearing again.
[snip]
>> + /*
>> + * In case of exynos, setting dma-burst to 16Word causes permanent
>> + * tearing for very small buffers, e.g. cursor buffer. Burst Mode
>> + * switching which is based on overlay size is not recommended as
>> + * overlay size varies alot towards the end of the screen and rapid
>> + * movement causes unstable DMA which results into iommu crash/tear.
>
> FIMD has width limitation so width of hardware overlay may need to be
> aligned to 16 pixels.
> We had faced with similar issue and the issue had been resolved by
> aligning it to 16 pixels.
>
> So can you try to align it instead of changing burst len size?
>
This problem is only with the very small FBs which are
rendered towards corners. For large FBs like 1366x768,
we dont see this issue though 1366 is not 16 pixel
aligned.
But I still carried out following experiments just to
verify the fimd behaviour for small overlay.
1) Round DOWN:
+ overlay->crtc_width = ((overlay->crtc_width)/16)*16;
Cursor corruption from A to B
A: [ 308.53] start addr = 0x20408000, end addr = 0x2040e000, size = 0x6000
[ 308.53] ovl_width = 64, ovl_height = 64
[ 308.54] offset_x = 1302, offset_y = 335
[ 308.54] fb_width = 96, fb_height = 64
B: [ 341.89] start addr = 0x20408000, end addr = 0x2040e000, size = 0x6000
[ 341.89] ovl_width = 48, ovl_height = 64
[ 341.90] offset_x = 1303, offset_y = 335
[ 341.90] fb_width = 96, fb_height = 64
2) Round UP:
+ overlay->crtc_width = ((overlay->crtc_width + 16)/16)*16;
+ if(overlay->crtc_width > overlay->fb_width)
+ overlay->crtc_width = overlay->fb_width;
Cursor corruption from A to B
A: [ 63.20] start addr = 0x20408000, end addr = 0x2040e000, size = 0x6000
[ 63.21] ovl_width = 96, ovl_height = 64
[ 63.21] offset_x = 1271, offset_y = 346
[ 63.22] fb_width = 96, fb_height = 64
B: [ 68.60] start addr = 0x20408000, end addr = 0x2040e000, size = 0x6000
[ 68.61] ovl_width = 96, ovl_height = 64
[ 68.61] offset_x = 1272, offset_y = 346
[ 68.62] fb_width = 96, fb_height = 64
In both the scenarios ovl_width is always 16 Pixel aligned.
Please let me know if you have something else in mind. I
will check and share the observations.
The solution in the patch is proposed by hardware team
and work fine for exynos 5250, 5420 and 5800.
Regards,
Rahul Sharma
> Thanks,
> Inki Dae
>
>> + */
>> +
>> + if (win_data->fb_width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
>> + val &= ~WINCONx_BURSTLEN_MASK;
>> + val |= WINCONx_BURSTLEN_4WORD;
>> + }
>> +
>> writel(val, ctx->regs + WINCON(win));
>> }
>>
>>
>
[PATCH v2] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
Hi Inki, Tomasz,
Any comment on this patch?
Regards,
Rahul Sharma
On 20 May 2014 10:36, Rahul Sharma wrote:
> Exynos drm hdmi driver used to get dummy hdmiphy clock to
> control the PMU bit for hdmiphy. This bit needs to be set
> before setting any resolution to hdmi hardware. This was
> handled using dummy hdmiphy clock which is removed here.
>
> PMU is already defined as system controller for exynos
> SoCs. Hdmi driver is modified to control the phy enable bit
> inside PMU using regmap interfaces.
>
> Devicetree binding document for hdmi is also updated.
>
> Signed-off-by: Rahul Sharma
> ---
> V2:
> 1) Squashed hdmiphy clock cleanup patch.
> 2) Addressed comments related to indentation, using
> BIT macro while definnig bits and using IS_ERR check
> while verifying regmap handle.
>
> This patch is based on exynos-drm-next branch.
>
> .../devicetree/bindings/video/exynos_hdmi.txt |2 ++
> drivers/gpu/drm/exynos/exynos_hdmi.c | 27
> ++--
> drivers/gpu/drm/exynos/regs-hdmi.h |4 +++
> 3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> index 75ada04..1fd8cf9 100644
> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> @@ -28,6 +28,7 @@ Required properties:
> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
> - ddc: phandle to the hdmi ddc node
> - phy: phandle to the hdmi phy node
> +- samsung,syscon-phandle: phandle for system controller node for PMU.
>
> Example:
>
> @@ -38,4 +39,5 @@ Example:
> hpd-gpio = <&gpx3 7 1>;
> ddc = <&hdmi_ddc_node>;
> phy = <&hdmi_phy_node>;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> };
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index b03e721..f5e188f 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -38,6 +38,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
>
> @@ -81,7 +83,6 @@ struct hdmi_resources {
> struct clk *sclk_hdmi;
> struct clk *sclk_pixel;
> struct clk *sclk_hdmiphy;
> - struct clk *hdmiphy;
> struct clk *mout_hdmi;
> struct regulator_bulk_data *regul_bulk;
> int regul_count;
> @@ -208,6 +209,7 @@ struct hdmi_context {
> const struct hdmiphy_config *phy_confs;
> unsigned intphy_conf_count;
>
> + struct regmap *pmureg;
> enum hdmi_type type;
> };
>
> @@ -2013,7 +2015,10 @@ static void hdmi_poweron(struct exynos_drm_display
> *display)
> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>
> - clk_prepare_enable(res->hdmiphy);
> + /* set pmu hdmiphy control bit to enable hdmiphy */
> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
> + PMU_HDMI_PHY_ENABLE_BIT, 1);
> +
> clk_prepare_enable(res->hdmi);
> clk_prepare_enable(res->sclk_hdmi);
>
> @@ -2040,7 +2045,11 @@ static void hdmi_poweroff(struct exynos_drm_display
> *display)
>
> clk_disable_unprepare(res->sclk_hdmi);
> clk_disable_unprepare(res->hdmi);
> - clk_disable_unprepare(res->hdmiphy);
> +
> + /* reset pmu hdmiphy control bit to disable hdmiphy */
> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
> + PMU_HDMI_PHY_ENABLE_BIT, 0);
> +
> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>
> pm_runtime_put_sync(hdata->dev);
> @@ -2143,11 +2152,6 @@ static int hdmi_resources_init(struct hdmi_context
> *hdata)
> DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
> goto fail;
> }
> - res->hdmiphy = devm_clk_get(dev, "hdmiphy");
> - if (IS_ERR(res->hdmiphy)) {
> - DRM_ERROR("failed to get clock 'hdmiphy'\n");
> - goto fail;
> - }
>
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 19 May 2014 16:24, Tomasz Figa wrote:
> On 19.05.2014 09:10, Rahul Sharma wrote:
>> On 16 May 2014 20:19, Tomasz Figa wrote:
>>> On 16.05.2014 16:30, Rahul Sharma wrote:
>>>> On 16 May 2014 16:20, Tomasz Figa wrote:
>>>>> On 16.05.2014 12:35, Rahul Sharma wrote:
>>>>>> On 16 May 2014 15:12, Rahul Sharma wrote:
>>>>>>> On 16 May 2014 03:14, Tomasz Figa wrote:
>>>>>>>> On 15.05.2014 06:01, Rahul Sharma wrote:
>>>>>> [snip]
>>>>>>>>>> the PHY provider.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please correct me if I got you wrong. You want somthing like this:
>>>>>>>>>
>>>>>>>>> pmu_system_controller: system-controller at 1004 {
>>>>>>>>> ...
>>>>>>>>> simple_phys: simple-phys {
>>>>>>>>> compatible = "samsung,exynos5420-simple-phy";
>>>>>>>>> ...
>>>>>>>>> };
>>>>>>>>> };
>>>>>>>>
>>>>>>>> Not exactly.
>>>>>>>>
>>>>>>>> What I meant is that the PMU node itself should be the PHY provider,
>>>>>>>> e.g.
>>>>>>>>
>>>>>>>> pmu_system_controller: system-controller at 1004 {
>>>>>>>> /* ... */
>>>>>>>> #phy-cells = <1>;
>>>>>>>> };
>>>>>>>>
>>>>>>>> and then the PMU node should instantiate the Exynos simple PHY driver,
>>>>>>>> as this is a driver for a facility existing entirely inside of the PMU.
>>>>>>>> Moreover, the driver should be rather called Exynos PMU PHY.
>>>>>>>>
>>>>>>>> I know this isn't really possible at the moment, but with device tree
>>>>>>>> we
>>>>>>>> must design things carefully, so it's better to take a bit more time
>>>>>>>> and
>>>>>>>> do things properly.
>>>>>>>>
>>>>>>>> So my opinion on this is that there should be a central Exynos PMU
>>>>>>>> driver that claims the IO region and instantiates necessary subdrivers,
>>>>>>>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>>>>>>>> driver and more, similar to what is being done in drivers/mfd.
>>>>>>>
>>>>>>
>>>>>> Hi Tomasz,
>>>>>>
>>>>>> These PHYs are not part of PMU as such. I am not sure if it is correct to
>>>>>> probe them as phy provider for all these phys. Only relation of these
>>>>>> phys with
>>>>>> the PMU is 'enable/disable control'.
>>>>>
>>>>> Well, in reality what is implemented by this driver is not even a PHY,
>>>>> just some kind of power controllers, which are contained entirely in the
>>>>> PMU.
>>>>>
>>>>
>>>> I agree. Actually the role of generic phy framework for these 'simple'
>>>> phys is
>>>> only that much.
>>>>
>>>>>> Controlling this bit using regmap interface
>>>>>> still looks better to me.
>>>>>
>>>>> Well, when there is a choice between using regmap and not using regmap,
>>>>> I'd rather choose the latter. Why would you want to introduce additional
>>>>> abstraction layer if there is no need for such?
>>>>>
>>>>>>
>>>>>> IMHO Ideal method would be probing these PHYs independently and resolving
>>>>>> the necessary dependencies like syscon handle, clocks etc. This way we
>>>>>> will
>>>>>> not be having any common phy provider for all these independent PHYs and
>>>>>> it
>>>>>> would be clean to add each of these phy nodes in DT. Please see my
>>>>>> original
>>>>>> comment below.
>>>>>>
>>>>>> http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
>>>>>
>>>>> With the solution I proposed, you don't need any kind of dependencies
>>>>> for those simple power controllers. They are just single bits that don't
>>>>> need anything special to operate, except PMU clock running.
>>>>
>>>> In that case we can further trim it down and let the drivers use the regmap
>>>> interface to control this bit. Many drivers including HDMI, DP just need
>>>> that
>>>> much functionality from the phy provider.
>>>
>>> Well, this is what several drivers already do, like USB PHY (dedicated
>>> IP block), watchdog (for watchdog mask), SATA PHY (dedicated IP block
>>> too) or will do, like I2C (for configuration of I2C mux on Exynos5).
>>>
>>> At least this would be consistent with them and wouldn't be an API
>>> abuse, so I'd be inclined to go this way more than introducing
>>> abstractions like this patch does.
>>
>> Ok. I had already posted a patch for this at
>> http://www.spinics.net/lists/linux-samsung-soc/msg28049.html
>> I will revive that thread.
>
> Looks good to me.
>
>>
>> @Tomasz Stanislawski, Do you have different opinion here?
>
> I'm afraid Tomasz might not be very responsive during next few days, as
> he is on a business trip. You might be able to reach him on our internal
> communicator, though.
Thanks Tomasz,
I will contact him over communicator.
Regards.
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
Exynos drm hdmi driver used to get dummy hdmiphy clock to
control the PMU bit for hdmiphy. This bit needs to be set
before setting any resolution to hdmi hardware. This was
handled using dummy hdmiphy clock which is removed here.
PMU is already defined as system controller for exynos
SoCs. Hdmi driver is modified to control the phy enable bit
inside PMU using regmap interfaces.
Devicetree binding document for hdmi is also updated.
Signed-off-by: Rahul Sharma
---
V2:
1) Squashed hdmiphy clock cleanup patch.
2) Addressed comments related to indentation, using
BIT macro while definnig bits and using IS_ERR check
while verifying regmap handle.
This patch is based on exynos-drm-next branch.
.../devicetree/bindings/video/exynos_hdmi.txt |2 ++
drivers/gpu/drm/exynos/exynos_hdmi.c | 27 ++--
drivers/gpu/drm/exynos/regs-hdmi.h |4 +++
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
index 75ada04..1fd8cf9 100644
--- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
+++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
@@ -28,6 +28,7 @@ Required properties:
"hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
- ddc: phandle to the hdmi ddc node
- phy: phandle to the hdmi phy node
+- samsung,syscon-phandle: phandle for system controller node for PMU.
Example:
@@ -38,4 +39,5 @@ Example:
hpd-gpio = <&gpx3 7 1>;
ddc = <&hdmi_ddc_node>;
phy = <&hdmi_phy_node>;
+ samsung,syscon-phandle = <&pmu_system_controller>;
};
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index b03e721..f5e188f 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -38,6 +38,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -81,7 +83,6 @@ struct hdmi_resources {
struct clk *sclk_hdmi;
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
- struct clk *hdmiphy;
struct clk *mout_hdmi;
struct regulator_bulk_data *regul_bulk;
int regul_count;
@@ -208,6 +209,7 @@ struct hdmi_context {
const struct hdmiphy_config *phy_confs;
unsigned intphy_conf_count;
+ struct regmap *pmureg;
enum hdmi_type type;
};
@@ -2013,7 +2015,10 @@ static void hdmi_poweron(struct exynos_drm_display
*display)
if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
DRM_DEBUG_KMS("failed to enable regulator bulk\n");
- clk_prepare_enable(res->hdmiphy);
+ /* set pmu hdmiphy control bit to enable hdmiphy */
+ regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
+ PMU_HDMI_PHY_ENABLE_BIT, 1);
+
clk_prepare_enable(res->hdmi);
clk_prepare_enable(res->sclk_hdmi);
@@ -2040,7 +2045,11 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
clk_disable_unprepare(res->sclk_hdmi);
clk_disable_unprepare(res->hdmi);
- clk_disable_unprepare(res->hdmiphy);
+
+ /* reset pmu hdmiphy control bit to disable hdmiphy */
+ regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
+ PMU_HDMI_PHY_ENABLE_BIT, 0);
+
regulator_bulk_disable(res->regul_count, res->regul_bulk);
pm_runtime_put_sync(hdata->dev);
@@ -2143,11 +2152,6 @@ static int hdmi_resources_init(struct hdmi_context
*hdata)
DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
goto fail;
}
- res->hdmiphy = devm_clk_get(dev, "hdmiphy");
- if (IS_ERR(res->hdmiphy)) {
- DRM_ERROR("failed to get clock 'hdmiphy'\n");
- goto fail;
- }
res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
if (IS_ERR(res->mout_hdmi)) {
DRM_ERROR("failed to get clock 'mout_hdmi'\n");
@@ -2353,6 +2357,13 @@ static int hdmi_probe(struct platform_device *pdev)
goto err_hdmiphy;
}
+ hdata->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "samsung,syscon-phandle");
+ if (IS_ERR(hdata->pmureg)) {
+ DRM_ERROR("syscon regmap lookup failed.\n");
+ goto err_hdmiphy;
+ }
+
pm_runtime_enable(dev);
hdmi_display.ctx =
[PATCH 2/5] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
On 11 April 2014 07:22, Rahul Sharma wrote:
> Thanks Tomasz,
>
> This patch is not longer required after rebasing to Tomasz Stanislawski's
> Simple Phy patches.
>
Hi All,
We had further discussion on the "Simple Phy Driver" at
http://www.spinics.net/lists/linux-samsung-soc/msg31100.html. The
discussion went in favor of the use of regmap interface.
As many other drivers are already using regmap interfaces for the same
purpose, it would also be consistent to use PMU syscon handle to control
phy enable bit in the hdmi driver. I will address the pending comments from
Tomasz Figa and post the next version of this patch.
Please let me know if any concern.
Regards,
Rahul Sharma
> Regards,
> Rahul Sharma.
>
> On 10 April 2014 22:30, Tomasz Figa wrote:
>> Hi Rahul,
>>
>> On 02.04.2014 19:13, Rahul Sharma wrote:
>>>
>>> From: Rahul Sharma
>>>
>>> Hdmiphy control bit needs to be set before setting the resolution
>>> to hdmi hardware. This was handled using dummy hdmiphy clock which
>>> is removed now.
>>>
>>> PMU is already defined as system controller for exynos SoC. Registers
>>> of PMU are accessed using regmap interfaces.
>>>
>>> Devicetree binding document for hdmi is also updated.
>>>
>>> Signed-off-by: Rahul Sharma
>>> ---
>>> .../devicetree/bindings/video/exynos_hdmi.txt |2 ++
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 17
>>> +
>>> drivers/gpu/drm/exynos/regs-hdmi.h |4
>>> 3 files changed, 23 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> index f9187a2..243a499 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> @@ -27,6 +27,7 @@ Required properties:
>>> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
>>> - ddc: phandle to the hdmi ddc node
>>> - phy: phandle to the hdmi phy node
>>> +- samsung,syscon-phandle: phandle for system controller node for PMU.
>>>
>>> Example:
>>>
>>> @@ -37,4 +38,5 @@ Example:
>>> hpd-gpio = <&gpx3 7 1>;
>>> ddc = <&hdmi_ddc_node>;
>>> phy = <&hdmi_phy_node>;
>>> + samsung,syscon-phandle = <&pmu_system_controller>;
>>> };
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index 23abfa0..47b8c06 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -36,6 +36,8 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> +#include
>>>
>>> #include
>>>
>>> @@ -194,6 +196,7 @@ struct hdmi_context {
>>> struct hdmi_resources res;
>>>
>>> int hpd_gpio;
>>> + struct regmap *pmureg;
>>>
>>> enum hdmi_type type;
>>> };
>>> @@ -1853,6 +1856,9 @@ static void hdmi_poweron(struct exynos_drm_display
>>> *display)
>>> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>>> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>>>
>>> + /* set pmu hdmiphy control bit to enable hdmiphy */
>>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>>> + PMU_HDMI_PHY_ENABLE_BIT, 1);
>>> clk_prepare_enable(res->hdmi);
>>> clk_prepare_enable(res->sclk_hdmi);
>>>
>>> @@ -1879,6 +1885,10 @@ static void hdmi_poweroff(struct exynos_drm_display
>>> *display)
>>>
>>> clk_disable_unprepare(res->sclk_hdmi);
>>> clk_disable_unprepare(res->hdmi);
>>
>>
>> nit: Blank line would beautify the code a bit.
>>
>>> + /* reset pmu hdmiphy control bit to disable hdmiphy */
>>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>>> + PMU_HDMI_PHY_ENABLE_BIT, 0);
>>> +
>>> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>>>
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 16 May 2014 20:19, Tomasz Figa wrote:
> On 16.05.2014 16:30, Rahul Sharma wrote:
>> On 16 May 2014 16:20, Tomasz Figa wrote:
>>> On 16.05.2014 12:35, Rahul Sharma wrote:
>>>> On 16 May 2014 15:12, Rahul Sharma wrote:
>>>>> On 16 May 2014 03:14, Tomasz Figa wrote:
>>>>>> On 15.05.2014 06:01, Rahul Sharma wrote:
>>>> [snip]
>>>>>>>> the PHY provider.
>>>>>>>>
>>>>>>>
>>>>>>> Please correct me if I got you wrong. You want somthing like this:
>>>>>>>
>>>>>>> pmu_system_controller: system-controller at 1004 {
>>>>>>> ...
>>>>>>> simple_phys: simple-phys {
>>>>>>> compatible = "samsung,exynos5420-simple-phy";
>>>>>>> ...
>>>>>>> };
>>>>>>> };
>>>>>>
>>>>>> Not exactly.
>>>>>>
>>>>>> What I meant is that the PMU node itself should be the PHY provider, e.g.
>>>>>>
>>>>>> pmu_system_controller: system-controller at 1004 {
>>>>>> /* ... */
>>>>>> #phy-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> and then the PMU node should instantiate the Exynos simple PHY driver,
>>>>>> as this is a driver for a facility existing entirely inside of the PMU.
>>>>>> Moreover, the driver should be rather called Exynos PMU PHY.
>>>>>>
>>>>>> I know this isn't really possible at the moment, but with device tree we
>>>>>> must design things carefully, so it's better to take a bit more time and
>>>>>> do things properly.
>>>>>>
>>>>>> So my opinion on this is that there should be a central Exynos PMU
>>>>>> driver that claims the IO region and instantiates necessary subdrivers,
>>>>>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>>>>>> driver and more, similar to what is being done in drivers/mfd.
>>>>>
>>>>
>>>> Hi Tomasz,
>>>>
>>>> These PHYs are not part of PMU as such. I am not sure if it is correct to
>>>> probe them as phy provider for all these phys. Only relation of these phys
>>>> with
>>>> the PMU is 'enable/disable control'.
>>>
>>> Well, in reality what is implemented by this driver is not even a PHY,
>>> just some kind of power controllers, which are contained entirely in the
>>> PMU.
>>>
>>
>> I agree. Actually the role of generic phy framework for these 'simple' phys
>> is
>> only that much.
>>
>>>> Controlling this bit using regmap interface
>>>> still looks better to me.
>>>
>>> Well, when there is a choice between using regmap and not using regmap,
>>> I'd rather choose the latter. Why would you want to introduce additional
>>> abstraction layer if there is no need for such?
>>>
>>>>
>>>> IMHO Ideal method would be probing these PHYs independently and resolving
>>>> the necessary dependencies like syscon handle, clocks etc. This way we will
>>>> not be having any common phy provider for all these independent PHYs and it
>>>> would be clean to add each of these phy nodes in DT. Please see my original
>>>> comment below.
>>>>
>>>> http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
>>>
>>> With the solution I proposed, you don't need any kind of dependencies
>>> for those simple power controllers. They are just single bits that don't
>>> need anything special to operate, except PMU clock running.
>>
>> In that case we can further trim it down and let the drivers use the regmap
>> interface to control this bit. Many drivers including HDMI, DP just need that
>> much functionality from the phy provider.
>
> Well, this is what several drivers already do, like USB PHY (dedicated
> IP block), watchdog (for watchdog mask), SATA PHY (dedicated IP block
> too) or will do, like I2C (for configuration of I2C mux on Exynos5).
>
> At least this would be consistent with them and wouldn't be an API
> abuse, so I'd be inclined to go this way more than introducing
> abstractions like this patch does.
Ok. I had already posted a patch for this at
http://www.spinics.net/lists/linux-samsung-soc/msg28049.html
I will revive that thread.
@Tomasz Stanislawski, Do you have different opinion here?
Regards,
Rahul Sharma.
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 16 May 2014 16:20, Tomasz Figa wrote:
> On 16.05.2014 12:35, Rahul Sharma wrote:
>> On 16 May 2014 15:12, Rahul Sharma wrote:
>>> On 16 May 2014 03:14, Tomasz Figa wrote:
>>>> On 15.05.2014 06:01, Rahul Sharma wrote:
>> [snip]
>>>>>> the PHY provider.
>>>>>>
>>>>>
>>>>> Please correct me if I got you wrong. You want somthing like this:
>>>>>
>>>>> pmu_system_controller: system-controller at 1004 {
>>>>> ...
>>>>> simple_phys: simple-phys {
>>>>> compatible = "samsung,exynos5420-simple-phy";
>>>>> ...
>>>>> };
>>>>> };
>>>>
>>>> Not exactly.
>>>>
>>>> What I meant is that the PMU node itself should be the PHY provider, e.g.
>>>>
>>>> pmu_system_controller: system-controller at 1004 {
>>>> /* ... */
>>>> #phy-cells = <1>;
>>>> };
>>>>
>>>> and then the PMU node should instantiate the Exynos simple PHY driver,
>>>> as this is a driver for a facility existing entirely inside of the PMU.
>>>> Moreover, the driver should be rather called Exynos PMU PHY.
>>>>
>>>> I know this isn't really possible at the moment, but with device tree we
>>>> must design things carefully, so it's better to take a bit more time and
>>>> do things properly.
>>>>
>>>> So my opinion on this is that there should be a central Exynos PMU
>>>> driver that claims the IO region and instantiates necessary subdrivers,
>>>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>>>> driver and more, similar to what is being done in drivers/mfd.
>>>
>>
>> Hi Tomasz,
>>
>> These PHYs are not part of PMU as such. I am not sure if it is correct to
>> probe them as phy provider for all these phys. Only relation of these phys
>> with
>> the PMU is 'enable/disable control'.
>
> Well, in reality what is implemented by this driver is not even a PHY,
> just some kind of power controllers, which are contained entirely in the
> PMU.
>
I agree. Actually the role of generic phy framework for these 'simple' phys is
only that much.
>> Controlling this bit using regmap interface
>> still looks better to me.
>
> Well, when there is a choice between using regmap and not using regmap,
> I'd rather choose the latter. Why would you want to introduce additional
> abstraction layer if there is no need for such?
>
>>
>> IMHO Ideal method would be probing these PHYs independently and resolving
>> the necessary dependencies like syscon handle, clocks etc. This way we will
>> not be having any common phy provider for all these independent PHYs and it
>> would be clean to add each of these phy nodes in DT. Please see my original
>> comment below.
>>
>> http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
>
> With the solution I proposed, you don't need any kind of dependencies
> for those simple power controllers. They are just single bits that don't
> need anything special to operate, except PMU clock running.
In that case we can further trim it down and let the drivers use the regmap
interface to control this bit. Many drivers including HDMI, DP just need that
much functionality from the phy provider.
Regards,
Rahul Sharma
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 16 May 2014 15:12, Rahul Sharma wrote:
> On 16 May 2014 03:14, Tomasz Figa wrote:
>> On 15.05.2014 06:01, Rahul Sharma wrote:
[snip]
>>>> the PHY provider.
>>>>
>>>
>>> Please correct me if I got you wrong. You want somthing like this:
>>>
>>> pmu_system_controller: system-controller at 1004 {
>>> ...
>>> simple_phys: simple-phys {
>>> compatible = "samsung,exynos5420-simple-phy";
>>> ...
>>> };
>>> };
>>
>> Not exactly.
>>
>> What I meant is that the PMU node itself should be the PHY provider, e.g.
>>
>> pmu_system_controller: system-controller at 1004 {
>> /* ... */
>> #phy-cells = <1>;
>> };
>>
>> and then the PMU node should instantiate the Exynos simple PHY driver,
>> as this is a driver for a facility existing entirely inside of the PMU.
>> Moreover, the driver should be rather called Exynos PMU PHY.
>>
>> I know this isn't really possible at the moment, but with device tree we
>> must design things carefully, so it's better to take a bit more time and
>> do things properly.
>>
>> So my opinion on this is that there should be a central Exynos PMU
>> driver that claims the IO region and instantiates necessary subdrivers,
>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>> driver and more, similar to what is being done in drivers/mfd.
>
Hi Tomasz,
These PHYs are not part of PMU as such. I am not sure if it is correct to
probe them as phy provider for all these phys. Only relation of these phys with
the PMU is 'enable/disable control'. Controlling this bit using regmap interface
still looks better to me.
IMHO Ideal method would be probing these PHYs independently and resolving
the necessary dependencies like syscon handle, clocks etc. This way we will
not be having any common phy provider for all these independent PHYs and it
would be clean to add each of these phy nodes in DT. Please see my original
comment below.
http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
Regards,
Rahul Sharma
>
>>
>> Now, there is already ongoing effort to convert current freestanding PMU
>> configuration code in mach-exynos into a full-fledged PMU driver, but
>> not exactly in the same direction as I stated above. I'll try to
>> cooperate with Pankaj, who is responsible for this work to make this go
>> into the right track.
>>
>> Best regards,
>> Tomasz
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 16 May 2014 03:14, Tomasz Figa wrote:
> On 15.05.2014 06:01, Rahul Sharma wrote:
>> Thanks Tomasz,
>>
>> On 15 May 2014 01:31, Tomasz Figa wrote:
>>> Hi Rahul, Tomasz,
>> [snip]
>>>> + simplephys: simple-phys at 1004 {
>>>> + compatible = "samsung,exynos5250-simple-phy";
>>>
>>> Missing reg property or unnecessary @unit-address suffix in node name.
>>
>> I should have removed "@unit-address". I will change this.
>>
>>>
>>>> + samsung,pmu-syscon = <&pmu_system_controller>;
>>>> + #phy-cells = <1>;
>>>> + };
>>>
>>> In general, the idea is quite good, but I think this should rather bind
>>> to the main PMU node, since this is just a part of the PMU, not another
>>> device in the system. This also means that the PMU node itself should be
>>> the PHY provider.
>>>
>>
>> Please correct me if I got you wrong. You want somthing like this:
>>
>> pmu_system_controller: system-controller at 1004 {
>> ...
>> simple_phys: simple-phys {
>> compatible = "samsung,exynos5420-simple-phy";
>> ...
>> };
>> };
>
> Not exactly.
>
> What I meant is that the PMU node itself should be the PHY provider, e.g.
>
> pmu_system_controller: system-controller at 1004 {
> /* ... */
> #phy-cells = <1>;
> };
>
> and then the PMU node should instantiate the Exynos simple PHY driver,
> as this is a driver for a facility existing entirely inside of the PMU.
> Moreover, the driver should be rather called Exynos PMU PHY.
>
> I know this isn't really possible at the moment, but with device tree we
> must design things carefully, so it's better to take a bit more time and
> do things properly.
>
> So my opinion on this is that there should be a central Exynos PMU
> driver that claims the IO region and instantiates necessary subdrivers,
> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
> driver and more, similar to what is being done in drivers/mfd.
Hi Tomasz,
>
> Now, there is already ongoing effort to convert current freestanding PMU
> configuration code in mach-exynos into a full-fledged PMU driver, but
> not exactly in the same direction as I stated above. I'll try to
> cooperate with Pankaj, who is responsible for this work to make this go
> into the right track.
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 15 May 2014 19:11, Kishon Vijay Abraham I wrote: > > > On Thursday 15 May 2014 07:05 PM, Rahul Sharma wrote: >> Hi, >> >> On 15 May 2014 19:01, Bartlomiej Zolnierkiewicz >> wrote: >>> >>> Hi, >>> >>> On Thursday, May 15, 2014 12:47:21 AM Rahul Sharma wrote: >>>> From: Tomasz Stanislawski >>>> >>>> Add exynos-simple-phy driver to support a single register >>>> PHY interfaces present on Exynos4 SoC. >>>> >>>> Signed-off-by: Tomasz Stanislawski >>>> Signed-off-by: Rahul Sharma >>>> >>>> --- >>>> .../devicetree/bindings/phy/samsung-phy.txt| 56 ++ >>>> drivers/phy/Kconfig|5 + >>>> drivers/phy/Makefile |1 + >>>> drivers/phy/exynos-simple-phy.c| 189 >>>> >>>> include/dt-bindings/phy/exynos-simple-phy.h| 27 +++ >>>> 5 files changed, 278 insertions(+) >>>> create mode 100644 drivers/phy/exynos-simple-phy.c >>>> create mode 100644 include/dt-bindings/phy/exynos-simple-phy.h >>> >>> [...] >>> >>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>> index 16a2f06..2a13e0d 100644 >>>> --- a/drivers/phy/Kconfig >>>> +++ b/drivers/phy/Kconfig >>>> @@ -178,4 +178,9 @@ config PHY_XGENE >>>> help >>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>> >>>> +config EXYNOS_SIMPLE_PHY >>>> + tristate "Exynos Simple PHY driver" >>> >>> Please limit this driver to EXYNOS platforms with: >>> >>> depends on ARCH_EXYNOS >>> >>> or (optionally) >>> >>> depends on ARCH_EXYNOS || COMPILE_TEST >>> >>> Moreover the generic PHY dependency is missing. It can be fixed with: >>> >>> select GENERIC_PHY >>> >> >> Yea, I will fix this part. > > While at that, change the header of the driver file to *2014* > Sure. Thanks, Rahul Sharma. > Thanks > Kishon >> >> Regards, >> Rahul Sharma. >> >>>> + help >>>> + Support for 1-bit PHY controllers on SoCs from Exynos family. >>>> + >>>> endmenu >>> >>> Best regards, >>> -- >>> Bartlomiej Zolnierkiewicz >>> Samsung R&D Institute Poland >>> Samsung Electronics >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> linux-samsung-soc" in >>> the body of a message to majordomo at 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 at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
Hi, On 15 May 2014 19:01, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Thursday, May 15, 2014 12:47:21 AM Rahul Sharma wrote: >> From: Tomasz Stanislawski >> >> Add exynos-simple-phy driver to support a single register >> PHY interfaces present on Exynos4 SoC. >> >> Signed-off-by: Tomasz Stanislawski >> Signed-off-by: Rahul Sharma >> >> --- >> .../devicetree/bindings/phy/samsung-phy.txt| 56 ++ >> drivers/phy/Kconfig|5 + >> drivers/phy/Makefile |1 + >> drivers/phy/exynos-simple-phy.c| 189 >> >> include/dt-bindings/phy/exynos-simple-phy.h| 27 +++ >> 5 files changed, 278 insertions(+) >> create mode 100644 drivers/phy/exynos-simple-phy.c >> create mode 100644 include/dt-bindings/phy/exynos-simple-phy.h > > [...] > >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index 16a2f06..2a13e0d 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -178,4 +178,9 @@ config PHY_XGENE >> help >> This option enables support for APM X-Gene SoC multi-purpose PHY. >> >> +config EXYNOS_SIMPLE_PHY >> + tristate "Exynos Simple PHY driver" > > Please limit this driver to EXYNOS platforms with: > > depends on ARCH_EXYNOS > > or (optionally) > > depends on ARCH_EXYNOS || COMPILE_TEST > > Moreover the generic PHY dependency is missing. It can be fixed with: > > select GENERIC_PHY > Yea, I will fix this part. Regards, Rahul Sharma. >> + help >> + Support for 1-bit PHY controllers on SoCs from Exynos family. >> + >> endmenu > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/3] s5p-tv: hdmi: use hdmiphy as PHY
From: Tomasz Stanislawski
The HDMIPHY (physical interface) is controlled by a single
bit in a power controller's regiter. It was implemented
as clock. It was a simple but effective hack.
This patch makes S5P-HDMI driver to control HDMIPHY via PHY interface.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
drivers/media/platform/s5p-tv/hdmi_drv.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c
b/drivers/media/platform/s5p-tv/hdmi_drv.c
index 534722c..8013e52 100644
--- a/drivers/media/platform/s5p-tv/hdmi_drv.c
+++ b/drivers/media/platform/s5p-tv/hdmi_drv.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -66,7 +67,7 @@ struct hdmi_resources {
struct clk *sclk_hdmi;
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
- struct clk *hdmiphy;
+ struct phy *hdmiphy;
struct regulator_bulk_data *regul_bulk;
int regul_count;
};
@@ -586,7 +587,7 @@ static int hdmi_resource_poweron(struct hdmi_resources *res)
if (ret < 0)
return ret;
/* power-on hdmi physical interface */
- clk_enable(res->hdmiphy);
+ phy_power_on(res->hdmiphy);
/* use VPP as parent clock; HDMIPHY is not working yet */
clk_set_parent(res->sclk_hdmi, res->sclk_pixel);
/* turn clocks on */
@@ -600,7 +601,7 @@ static void hdmi_resource_poweroff(struct hdmi_resources
*res)
/* turn clocks off */
clk_disable(res->sclk_hdmi);
/* power-off hdmiphy */
- clk_disable(res->hdmiphy);
+ phy_power_off(res->hdmiphy);
/* turn HDMI power off */
regulator_bulk_disable(res->regul_count, res->regul_bulk);
}
@@ -784,7 +785,7 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev)
/* kfree is NULL-safe */
kfree(res->regul_bulk);
if (!IS_ERR(res->hdmiphy))
- clk_put(res->hdmiphy);
+ phy_put(res->hdmiphy);
if (!IS_ERR(res->sclk_hdmiphy))
clk_put(res->sclk_hdmiphy);
if (!IS_ERR(res->sclk_pixel))
@@ -835,7 +836,7 @@ static int hdmi_resources_init(struct hdmi_device *hdev)
dev_err(dev, "failed to get clock 'sclk_hdmiphy'\n");
goto fail;
}
- res->hdmiphy = clk_get(dev, "hdmiphy");
+ res->hdmiphy = phy_get(dev, "hdmiphy");
if (IS_ERR(res->hdmiphy)) {
dev_err(dev, "failed to get clock 'hdmiphy'\n");
goto fail;
--
1.7.9.5
[PATCH v4 2/3] drm: exynos: hdmi: use hdmiphy as PHY
From: Tomasz Stanislawski
The HDMIPHY (physical interface) is controlled by a single
bit in a power controller's regiter. It was implemented
as clock. It was a simple but effective hack.
This patch makes HDMI driver to control HDMIPHY via PHY interface.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 9a6d652..ef1cdd0 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
@@ -74,8 +75,8 @@ struct hdmi_resources {
struct clk *sclk_hdmi;
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
- struct clk *hdmiphy;
struct clk *mout_hdmi;
+ struct phy *hdmiphy;
struct regulator_bulk_data *regul_bulk;
int regul_count;
};
@@ -1854,7 +1855,7 @@ static void hdmi_poweron(struct exynos_drm_display
*display)
if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
DRM_DEBUG_KMS("failed to enable regulator bulk\n");
- clk_prepare_enable(res->hdmiphy);
+ phy_power_on(res->hdmiphy);
clk_prepare_enable(res->hdmi);
clk_prepare_enable(res->sclk_hdmi);
@@ -1881,7 +1882,7 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
clk_disable_unprepare(res->sclk_hdmi);
clk_disable_unprepare(res->hdmi);
- clk_disable_unprepare(res->hdmiphy);
+ phy_power_off(res->hdmiphy);
regulator_bulk_disable(res->regul_count, res->regul_bulk);
pm_runtime_put_sync(hdata->dev);
@@ -1977,9 +1978,9 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
goto fail;
}
- res->hdmiphy = devm_clk_get(dev, "hdmiphy");
+ res->hdmiphy = devm_phy_get(dev, "hdmiphy");
if (IS_ERR(res->hdmiphy)) {
- DRM_ERROR("failed to get clock 'hdmiphy'\n");
+ DRM_ERROR("failed to get phy 'hdmiphy'\n");
goto fail;
}
res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
--
1.7.9.5
[PATCH v4 1/3] phy: Add exynos-simple-phy driver
From: Tomasz Stanislawski
Add exynos-simple-phy driver to support a single register
PHY interfaces present on Exynos4 SoC.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
.../devicetree/bindings/phy/samsung-phy.txt| 57 ++
drivers/phy/Kconfig|5 +
drivers/phy/Makefile |1 +
drivers/phy/phy-exynos-simple.c| 189
include/dt-bindings/phy/phy-exynos-simple.h| 22 +++
5 files changed, 274 insertions(+)
create mode 100644 drivers/phy/phy-exynos-simple.c
create mode 100644 include/dt-bindings/phy/phy-exynos-simple.h
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 2049261..a8d95d6 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -161,3 +161,60 @@ Example:
usbdrdphy0 = &usb3_phy0;
usbdrdphy1 = &usb3_phy1;
};
+
+Samsung S5P/EXYNOS SoC series SIMPLE PHY
+-
+
+Required properties:
+- compatible : should be one of the listed compatibles:
+ - "samsung,exynos4210-simple-phy"
+ - "samsung,exynos4412-simple-phy"
+ - "samsung,exynos5250-simple-phy"
+ - "samsung,exynos5420-simple-phy"
+- samsung,pmureg-phandle - handle to syscon to control PMU registers
+- #phy-cells : from the generic phy bindings, must be 1;
+
+For a compatible PHY, the single cell specifier should be one of these
+listed values:
+
+ PHY_EXYNOS_SIMPLE_HDMI 0
+ PHY_EXYNOS_SIMPLE_DAC1
+ PHY_EXYNOS_SIMPLE_ADC2
+ PHY_EXYNOS_SIMPLE_PCIE 3
+ PHY_EXYNOS_SIMPLE_SATA 4
+
+List of supported PHYs for compatible "samsung,exynos4210-simple-phy" are:
+ PHY_EXYNOS_SIMPLE_HDMI,
+ PHY_EXYNOS_SIMPLE_DAC,
+ PHY_EXYNOS_SIMPLE_ADC,
+ PHY_EXYNOS_SIMPLE_PCIE,
+ PHY_EXYNOS_SIMPLE_SATA.
+
+List of supported PHYs for compatible "samsung,exynos4412-simple-phy" are:
+ PHY_EXYNOS_SIMPLE_HDMI,
+ PHY_EXYNOS_SIMPLE_ADC.
+
+List of supported PHYs for compatible "samsung,exynos5250-simple-phy" are:
+ PHY_EXYNOS_SIMPLE_HDMI,
+ PHY_EXYNOS_SIMPLE_ADC,
+ PHY_EXYNOS_SIMPLE_SATA.
+
+List of supported PHYs for compatible "samsung,exynos5420-simple-phy" are:
+ PHY_EXYNOS_SIMPLE_HDMI,
+ PHY_EXYNOS_SIMPLE_ADC.
+
+Example:
+Simple PHY provider node:
+
+ simplephys: simple-phys {
+ compatible = "samsung,exynos5250-simple-phy";
+ samsung,pmu-syscon = <&pmu_system_controller>;
+ #phy-cells = <1>;
+ };
+
+Other nodes accessing simple PHYs:
+
+ hdmi {
+ phys = <&simplephys PHY_EXYNOS_SIMPLE_HDMI>;
+ phy-names = "hdmiphy";
+ };
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f06..c306ff2 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -178,4 +178,9 @@ config PHY_XGENE
help
This option enables support for APM X-Gene SoC multi-purpose PHY.
+config PHY_EXYNOS_SIMPLE
+ tristate "Exynos Simple PHY driver"
+ help
+ Support for 1-bit PHY controllers on SoCs from Exynos family.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d57..cdf0b65 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_SIMPLE)+= phy-exynos-simple.o
obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
obj-$(CONFIG_OMAP_CONTROL_PHY) += phy-omap-control.o
obj-$(CONFIG_OMAP_USB2)+= phy-omap-usb2.o
diff --git a/drivers/phy/phy-exynos-simple.c b/drivers/phy/phy-exynos-simple.c
new file mode 100644
index 000..77fc3f7
--- /dev/null
+++ b/drivers/phy/phy-exynos-simple.c
@@ -0,0 +1,189 @@
+/*
+ * Exynos Simple PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tomasz Stanislawski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define EXYNOS_PHY_ENABLE (1 << 0)
+#define INVALID(~1)
+#define PHY_NR 5
+
+struct phy_driver_priv {
+ struct phy *phys[PHY_NR];
+ struct regmap *pmureg;
+ u32 offsets[PHY_NR];
+};
+
+struct phy
[PATCH v4 0/3] phy: Add exynos-simple-phy driver
From: Rahul Sharma Changelog: v4: * Rename files to follow "phy-exynos-simple" names. * Rename Macros to this convention PHY_EXYNOS_SIMPLE_. * Added list of phy specifier values to documentation file. * Removed of_match_ptr from driver probe. * Moved unrelated MACROs to driver file from header file. v3: * Implement lazy-init of PHYs. * Use MACROs instead of numbers to represent phys. * Use regmap interface to access PMU registers. It is based on "Next" branch in Kishon Vijay Abraham's tree at https://git.kernel.org/cgit/linux/kernel/git/kishon/linux-phy.git/ /* Original Message from Tomasz Stanislawski */ The Samsung SoCs from Exynos family are enhanced with a bunch of devices that provide functionality of a physical layer for interfaces like USB, HDMI, SATA, etc. They are controlled by a simple interface, often a single bit that enables and/or resets the physical layer. An IP driver should to control such a controller in an abstract manner. Therefore, such 'enablers' were implemented as clocks in older versions of Linux kernel. With the dawn of PHY subsystems, PHYs become a natural way of exporting the 'enabler' functionality to drivers. However, there is an unexpected consequence. Some of those 1-bit PHYs were implemented as separate drivers. This means that one has to create a struct device, struct phy, its phy provider and 100-150 lines of driver code to basically set one bit. The DP phy driver is a good example: https://lkml.org/lkml/2013/7/18/53 And simple-phy RFC (shares only driver code but not other resources): https://lkml.org/lkml/2013/10/21/313 To avoid waste of resources I propose to create all such 1-bit phys from Exynos SoC using a single device, driver and phy provider. This patchset contains a proposed solution. All comment are welcome. Hopefully in future the functionality introduced by this patch may be merged into a larger Power Management Unit (PMU) gluer driver. On Samsusng SoC , the PMU part contains a number of register barely linked to power management (like clock gating, clock dividers, CPU resetting, etc.). It may be tempting to create a hybrid driver that export clocks/phys/etc that are controlled by PMU unit. Alternative solutions might be: * exporting a regmap to the IP driver and allow the driver to control the PHY layer like in the patch: http://thread.gmane.org/gmane.linux.kernel.samsung-soc/28617/focus=28648 * create a dedicated power domain for hdmiphy Regards, Tomasz Stanislawski v2: * rename to exynos-simple-phy * fix usage of devm_ioremap() * add documentation for DT bindings * add patches to client drivers v1: initial version Tomasz Stanislawski (3): phy: Add exynos-simple-phy driver drm: exynos: hdmi: use hdmiphy as PHY s5p-tv: hdmi: use hdmiphy as PHY .../devicetree/bindings/phy/samsung-phy.txt| 57 ++ drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +- drivers/media/platform/s5p-tv/hdmi_drv.c | 11 +- drivers/phy/Kconfig|5 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos-simple.c| 189 include/dt-bindings/phy/phy-exynos-simple.h| 22 +++ 7 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 drivers/phy/phy-exynos-simple.c create mode 100644 include/dt-bindings/phy/phy-exynos-simple.h -- 1.7.9.5
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 15 May 2014 13:12, Thierry Reding wrote: > On Thu, May 15, 2014 at 10:49:37AM +0530, Rahul Sharma wrote: >> Hi Thierry, >> >> On 15 May 2014 03:44, Thierry Reding wrote: >> > On Thu, May 15, 2014 at 12:47:21AM +0530, Rahul Sharma wrote: > [...] >> >> +#define HDMI_PHY 0 >> >> +#define DAC_PHY 1 >> >> +#define ADC_PHY 2 >> >> +#define PCIE_PHY 3 >> >> +#define SATA_PHY 4 >> > >> > Perhaps these should be namespaced somehow to avoid potential conflicts >> > with other PHY providers? >> >> How about XXX_SIMPLE_PHY? > > The indices are specific to the Exynos PHY device, so why not > PHY_EXYNOS_SIMPLE_XXX (or any variation thereof)? This looks ok. I will use that. > >> >> +#define PHY_NR 5 >> > >> > I'm not sure that this belongs here either. It's not a value that will >> > ever appear in a DT source file. >> >> I want it to grow along with new additions in the phy list else >> catastrophic. This will look unrelated in driver. > > But this is in no way growing automatically as it is. Whoever adds a new > type of PHY will need to manually increment this define. Furthermore the > driver will need to be updated to cope with this anyway. Not automatically. What I meant was If keeping it at end of the list, it is not possible that somebody skip the updation of PHY_NR when adding a new phy type. If I leave a comment at the end of the list to update PHY_NR (after moving it to driver), that also serves the purpose. > > I think this is even a reason to have this only in the driver. Otherwise > you could have a situation where somebody upgrades the binding (and this > file along with it) but not update the driver with the necessary support. > But the driver will still pick up the PHY_NR change and will accept the > new PHY type as valid, even though it has no code to handle it. If you > have this in the driver, however, then as long as the driver has not yet > been updated it will reject requests for the new PHY type. And that's > exactly what it should be doing since it doesn't know how to handle it. hmn...Ok. Regards, Rahul Sharma > > Thierry
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
Hi Thierry,
On 15 May 2014 03:44, Thierry Reding wrote:
> On Thu, May 15, 2014 at 12:47:21AM +0530, Rahul Sharma wrote:
> [...]
>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> [...]
>> +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in
>> +the PHY specifier identifies the PHY and the supported phys for exynos4210
>
> I think the specifier is only the part after the phandle, so this should
> probably be "... compatible PHYs the single cell specifier ..." or
> something equivalent.
>
ok. I will rephrase this line.
>> +are:
>> + HDMI_PHY,
>> + DAC_PHY,
>> + ADC_PHY,
>> + PCIE_PHY,
>> + SATA_PHY.
>
> I think you need to specify the literal values here as well, since the
> binding must be fully self-contained. That is you can't rely on the DT
> binding to be bundled with the exynos-simple-phy.h header.
>
Ok. I will add that.
>> @@ -20,3 +20,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) +=
>> phy-exynos4x12-usb2.o
>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)+=
>> phy-exynos5250-usb2.o
>> obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
>> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
>> +obj-$(CONFIG_EXYNOS_SIMPLE_PHY) += exynos-simple-phy.o
>
> Perhaps this should be named phy-exynos-simple for consistency? Also it
> may be a good idea to sort this alphabetically to reduce the potential
> for conflicts.
yea correct. I will use "phy-exynos-simple".
I will place this addition in alphabetical order in makefile.
>
>> +static int exynos_phy_probe(struct platform_device *pdev)
>> +{
>> + const struct of_device_id *of_id = of_match_device(
>> + of_match_ptr(exynos_phy_of_match), &pdev->dev);
>
> Why does this need of_match_ptr()?
>
yea correct. Not required.
>> + dev_info(dev, "probe success\n");
>
> If at all this should be dev_dbg(). But in general the driver core will
> already complain if the driver fails to probe, so there's in general no
> need to mention when it probes successfully.
>
ok.
>> diff --git a/include/dt-bindings/phy/exynos-simple-phy.h
>> b/include/dt-bindings/phy/exynos-simple-phy.h
> [...]
>> +/* simeple phys */
>
> s/simeple phys/simple PHYs/
>
> Although on second thought that comment probably shouldn't be there in
> the first place.
>
>> +#define INVALID (~1)
>
> This doesn't belong in this header. The value should never be used by a
> DT source file, should it?
I will move this to driver file.
>
>> +#define HDMI_PHY 0
>> +#define DAC_PHY 1
>> +#define ADC_PHY 2
>> +#define PCIE_PHY 3
>> +#define SATA_PHY 4
>
> Perhaps these should be namespaced somehow to avoid potential conflicts
> with other PHY providers?
How about XXX_SIMPLE_PHY?
>
>> +#define PHY_NR 5
>
> I'm not sure that this belongs here either. It's not a value that will
> ever appear in a DT source file.
I want it to grow along with new additions in the phy list else
catastrophic. This will look unrelated in driver.
Regards,
Rahul Sharma.
>
> Thierry
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
Thanks Tomasz,
On 15 May 2014 01:31, Tomasz Figa wrote:
> Hi Rahul, Tomasz,
[snip]
>> + simplephys: simple-phys at 1004 {
>> + compatible = "samsung,exynos5250-simple-phy";
>
> Missing reg property or unnecessary @unit-address suffix in node name.
I should have removed "@unit-address". I will change this.
>
>> + samsung,pmu-syscon = <&pmu_system_controller>;
>> + #phy-cells = <1>;
>> + };
>
> In general, the idea is quite good, but I think this should rather bind
> to the main PMU node, since this is just a part of the PMU, not another
> device in the system. This also means that the PMU node itself should be
> the PHY provider.
>
Please correct me if I got you wrong. You want somthing like this:
pmu_system_controller: system-controller at 1004 {
...
simple_phys: simple-phys {
compatible = "samsung,exynos5420-simple-phy";
...
};
};
Regards,
Rahul Sharma.
> Otherwise looks good.
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] s5p-tv: hdmi: use hdmiphy as PHY
From: Tomasz Stanislawski
The HDMIPHY (physical interface) is controlled by a single
bit in a power controller's regiter. It was implemented
as clock. It was a simple but effective hack.
This patch makes S5P-HDMI driver to control HDMIPHY via PHY interface.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
drivers/media/platform/s5p-tv/hdmi_drv.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c
b/drivers/media/platform/s5p-tv/hdmi_drv.c
index 534722c..8013e52 100644
--- a/drivers/media/platform/s5p-tv/hdmi_drv.c
+++ b/drivers/media/platform/s5p-tv/hdmi_drv.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -66,7 +67,7 @@ struct hdmi_resources {
struct clk *sclk_hdmi;
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
- struct clk *hdmiphy;
+ struct phy *hdmiphy;
struct regulator_bulk_data *regul_bulk;
int regul_count;
};
@@ -586,7 +587,7 @@ static int hdmi_resource_poweron(struct hdmi_resources *res)
if (ret < 0)
return ret;
/* power-on hdmi physical interface */
- clk_enable(res->hdmiphy);
+ phy_power_on(res->hdmiphy);
/* use VPP as parent clock; HDMIPHY is not working yet */
clk_set_parent(res->sclk_hdmi, res->sclk_pixel);
/* turn clocks on */
@@ -600,7 +601,7 @@ static void hdmi_resource_poweroff(struct hdmi_resources
*res)
/* turn clocks off */
clk_disable(res->sclk_hdmi);
/* power-off hdmiphy */
- clk_disable(res->hdmiphy);
+ phy_power_off(res->hdmiphy);
/* turn HDMI power off */
regulator_bulk_disable(res->regul_count, res->regul_bulk);
}
@@ -784,7 +785,7 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev)
/* kfree is NULL-safe */
kfree(res->regul_bulk);
if (!IS_ERR(res->hdmiphy))
- clk_put(res->hdmiphy);
+ phy_put(res->hdmiphy);
if (!IS_ERR(res->sclk_hdmiphy))
clk_put(res->sclk_hdmiphy);
if (!IS_ERR(res->sclk_pixel))
@@ -835,7 +836,7 @@ static int hdmi_resources_init(struct hdmi_device *hdev)
dev_err(dev, "failed to get clock 'sclk_hdmiphy'\n");
goto fail;
}
- res->hdmiphy = clk_get(dev, "hdmiphy");
+ res->hdmiphy = phy_get(dev, "hdmiphy");
if (IS_ERR(res->hdmiphy)) {
dev_err(dev, "failed to get clock 'hdmiphy'\n");
goto fail;
--
1.7.9.5
[PATCH v3 2/3] drm: exynos: hdmi: use hdmiphy as PHY
From: Tomasz Stanislawski
The HDMIPHY (physical interface) is controlled by a single
bit in a power controller's regiter. It was implemented
as clock. It was a simple but effective hack.
This patch makes HDMI driver to control HDMIPHY via PHY interface.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 9a6d652..ef1cdd0 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
@@ -74,8 +75,8 @@ struct hdmi_resources {
struct clk *sclk_hdmi;
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
- struct clk *hdmiphy;
struct clk *mout_hdmi;
+ struct phy *hdmiphy;
struct regulator_bulk_data *regul_bulk;
int regul_count;
};
@@ -1854,7 +1855,7 @@ static void hdmi_poweron(struct exynos_drm_display
*display)
if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
DRM_DEBUG_KMS("failed to enable regulator bulk\n");
- clk_prepare_enable(res->hdmiphy);
+ phy_power_on(res->hdmiphy);
clk_prepare_enable(res->hdmi);
clk_prepare_enable(res->sclk_hdmi);
@@ -1881,7 +1882,7 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
clk_disable_unprepare(res->sclk_hdmi);
clk_disable_unprepare(res->hdmi);
- clk_disable_unprepare(res->hdmiphy);
+ phy_power_off(res->hdmiphy);
regulator_bulk_disable(res->regul_count, res->regul_bulk);
pm_runtime_put_sync(hdata->dev);
@@ -1977,9 +1978,9 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
goto fail;
}
- res->hdmiphy = devm_clk_get(dev, "hdmiphy");
+ res->hdmiphy = devm_phy_get(dev, "hdmiphy");
if (IS_ERR(res->hdmiphy)) {
- DRM_ERROR("failed to get clock 'hdmiphy'\n");
+ DRM_ERROR("failed to get phy 'hdmiphy'\n");
goto fail;
}
res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
--
1.7.9.5
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
From: Tomasz Stanislawski
Add exynos-simple-phy driver to support a single register
PHY interfaces present on Exynos4 SoC.
Signed-off-by: Tomasz Stanislawski
Signed-off-by: Rahul Sharma
---
.../devicetree/bindings/phy/samsung-phy.txt| 56 ++
drivers/phy/Kconfig|5 +
drivers/phy/Makefile |1 +
drivers/phy/exynos-simple-phy.c| 189
include/dt-bindings/phy/exynos-simple-phy.h| 27 +++
5 files changed, 278 insertions(+)
create mode 100644 drivers/phy/exynos-simple-phy.c
create mode 100644 include/dt-bindings/phy/exynos-simple-phy.h
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 2049261..12ad9cf 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -161,3 +161,59 @@ Example:
usbdrdphy0 = &usb3_phy0;
usbdrdphy1 = &usb3_phy1;
};
+
+Samsung S5P/EXYNOS SoC series SIMPLE PHY
+-
+
+Required properties:
+- compatible : should be one of the listed compatibles:
+ - "samsung,exynos4210-simple-phy"
+ - "samsung,exynos4412-simple-phy"
+ - "samsung,exynos5250-simple-phy"
+ - "samsung,exynos5420-simple-phy"
+- samsung,pmureg-phandle - handle to syscon to control PMU registers
+- #phy-cells : from the generic phy bindings, must be 1;
+
+For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and the supported phys for exynos4210
+are:
+ HDMI_PHY,
+ DAC_PHY,
+ ADC_PHY,
+ PCIE_PHY,
+ SATA_PHY.
+
+For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and the supported phys for exynos4412
+are:
+ HDMI_PHY,
+ ADC_PHY.
+
+For "samsung,exynos5250-simple-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and the supported phys for exynos5250
+are:
+ HDMI_PHY,
+ ADC_PHY,
+ SATA_PHY.
+
+For "samsung,exynos5420-simple-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and the supported phys for exynos5420
+are:
+ HDMI_PHY,
+ ADC_PHY.
+
+Example:
+Simple PHY provider node:
+
+ simplephys: simple-phys at 1004 {
+ compatible = "samsung,exynos5250-simple-phy";
+ samsung,pmu-syscon = <&pmu_system_controller>;
+ #phy-cells = <1>;
+ };
+
+Other nodes accessing simple PHYs:
+
+ hdmi {
+ phys = <&simplephys HDMI_PHY>;
+ phy-names = "hdmiphy";
+ };
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f06..2a13e0d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -178,4 +178,9 @@ config PHY_XGENE
help
This option enables support for APM X-Gene SoC multi-purpose PHY.
+config EXYNOS_SIMPLE_PHY
+ tristate "Exynos Simple PHY driver"
+ help
+ Support for 1-bit PHY controllers on SoCs from Exynos family.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d57..81b6efa 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -20,3 +20,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) +=
phy-exynos4x12-usb2.o
phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
obj-$(CONFIG_PHY_XGENE)+= phy-xgene.o
+obj-$(CONFIG_EXYNOS_SIMPLE_PHY)+= exynos-simple-phy.o
diff --git a/drivers/phy/exynos-simple-phy.c b/drivers/phy/exynos-simple-phy.c
new file mode 100644
index 000..792e9bc
--- /dev/null
+++ b/drivers/phy/exynos-simple-phy.c
@@ -0,0 +1,189 @@
+/*
+ * Exynos Simple PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tomasz Stanislawski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+struct phy_driver_priv {
+ struct phy *phys[PHY_NR];
+ struct regmap *pmureg;
+ u32 offsets[PHY_NR];
+};
+
+struct phy_driver_data {
+ u32 index;
+ u32 offset;
+};
+
+struct phy_private {
+ struct regmap *pmureg;
+ u32 offset;
+};
+
+#define EXYNOS_PHY_ENABLE (1 << 0)
+
+static int exynos_phy_power_on(struct phy *phy)
+{
+ struct phy_private *phy_private = phy_get_drvdata(phy);
+
+ return regmap_update_bits(phy_private->pmureg, phy_private->offset,
+ EXYNOS_PHY_ENABLE, 1);
+}
+
+static
[PATCH v3 0/3] phy: Add exynos-simple-phy driver
From: Rahul Sharma Hi All, This series has been originally proposed by Tomasz Stanislawski. With his permission, I have addressed the following review comments in V3. Changelog: v3: * Implement lazy-init of PHYs. * Use MACROs instead of numbers to represent phys. * Use regmap interface to access PMU registers. It is based on "Next" branch in Kishon Vijay Abraham's tree at https://git.kernel.org/cgit/linux/kernel/git/kishon/linux-phy.git/ /* Original Message from Tomasz Stanislawski */ The Samsung SoCs from Exynos family are enhanced with a bunch of devices that provide functionality of a physical layer for interfaces like USB, HDMI, SATA, etc. They are controlled by a simple interface, often a single bit that enables and/or resets the physical layer. An IP driver should to control such a controller in an abstract manner. Therefore, such 'enablers' were implemented as clocks in older versions of Linux kernel. With the dawn of PHY subsystems, PHYs become a natural way of exporting the 'enabler' functionality to drivers. However, there is an unexpected consequence. Some of those 1-bit PHYs were implemented as separate drivers. This means that one has to create a struct device, struct phy, its phy provider and 100-150 lines of driver code to basically set one bit. The DP phy driver is a good example: https://lkml.org/lkml/2013/7/18/53 And simple-phy RFC (shares only driver code but not other resources): https://lkml.org/lkml/2013/10/21/313 To avoid waste of resources I propose to create all such 1-bit phys from Exynos SoC using a single device, driver and phy provider. This patchset contains a proposed solution. All comment are welcome. Hopefully in future the functionality introduced by this patch may be merged into a larger Power Management Unit (PMU) gluer driver. On Samsusng SoC , the PMU part contains a number of register barely linked to power management (like clock gating, clock dividers, CPU resetting, etc.). It may be tempting to create a hybrid driver that export clocks/phys/etc that are controlled by PMU unit. Alternative solutions might be: * exporting a regmap to the IP driver and allow the driver to control the PHY layer like in the patch: http://thread.gmane.org/gmane.linux.kernel.samsung-soc/28617/focus=28648 * create a dedicated power domain for hdmiphy Regards, Tomasz Stanislawski v2: * rename to exynos-simple-phy * fix usage of devm_ioremap() * add documentation for DT bindings * add patches to client drivers v1: initial version Tomasz Stanislawski (3): phy: Add exynos-simple-phy driver drm: exynos: hdmi: use hdmiphy as PHY s5p-tv: hdmi: use hdmiphy as PHY .../devicetree/bindings/phy/samsung-phy.txt| 56 ++ drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +- drivers/media/platform/s5p-tv/hdmi_drv.c | 11 +- drivers/phy/Kconfig|5 + drivers/phy/Makefile |1 + drivers/phy/exynos-simple-phy.c| 189 include/dt-bindings/phy/exynos-simple-phy.h| 27 +++ 7 files changed, 290 insertions(+), 10 deletions(-) create mode 100644 drivers/phy/exynos-simple-phy.c create mode 100644 include/dt-bindings/phy/exynos-simple-phy.h -- 1.7.9.5
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
On 7 May 2014 21:03, Tomasz Figa wrote: > [CCing more DT-folks :)] > > On 07.05.2014 16:19, Rahul Sharma wrote: >> On 7 May 2014 19:06, Tomasz Stanislawski wrote: >>> On 05/07/2014 12:38 PM, Rahul Sharma wrote: >>>> On 5 May 2014 15:14, Kishon Vijay Abraham I wrote: >>>>> Hi, >>>>> >>>>> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>>>>> Hi, >>>>>> >>>>>> On 09/04/14 11:12, Rahul Sharma wrote: >>>>>>> Idea looks good. How about keeping compatible which is independent >>>>>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>>>>> and Bit through phy provider node. This way we can avoid SoC specific >>>>>>> hardcoding in phy driver and don't need to look into dt bindings for >>>>>>> each new SoC. >>>>>> >>>>>> I believe it is a not recommended approach. >>>>> >>>>> Why not? We should try to avoid hard coding in the driver code. Moreover >>>>> by >>>>> avoiding hardcoding we can make it a generic driver for single bit PHYs. >>>>> >>>> >>>> +1. >>>> >>>> @Tomasz, any plans to consider this approach for simple phy driver? >>>> >>>> Regards, >>>> Rahul Sharma. >>>> >>> >>> Hi Rahul, >>> Initially, I wanted to make a very generic driver and to add bit and >>> register (or its offset) attribute to the PHY node. >>> However, there was a very strong opposition from DT maintainers >>> to adding any bit related configuration to DT. >>> The current solution was designed to be a trade-off between >>> being generic and being accepted :). >>> >> >> Thanks Tomasz, >> Ok got it. lets discuss it again and conclude it. >> >> @Kishon, DT-folks, >> >> The original RFC patch from Tomasz (at https://lkml.org/lkml/2013/10/21/313) >> added simple phy driver as "Generic-simple-phy" with these properties: >> >> + of_property_read_u32(dev->of_node, "mask", &sphy->mask); >> + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); >> + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); >> >> Shall we consider the same solution again for generic simple phy >> driver which just expose on/off control through register bit. >> >> Regards, >> Rahul Sharma >> >>> Regards, >>> Tomasz Stanislawski >>> >>> >>> >>>>> Cheers >>>>> Kishon >>>> >>> >>> ___ >>> dri-devel mailing list >>> dri-devel at lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" >> in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > Best regards, > Tomasz > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
On 7 May 2014 19:06, Tomasz Stanislawski wrote: > On 05/07/2014 12:38 PM, Rahul Sharma wrote: >> On 5 May 2014 15:14, Kishon Vijay Abraham I wrote: >>> Hi, >>> >>> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>>> Hi, >>>> >>>> On 09/04/14 11:12, Rahul Sharma wrote: >>>>> Idea looks good. How about keeping compatible which is independent >>>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>>> and Bit through phy provider node. This way we can avoid SoC specific >>>>> hardcoding in phy driver and don't need to look into dt bindings for >>>>> each new SoC. >>>> >>>> I believe it is a not recommended approach. >>> >>> Why not? We should try to avoid hard coding in the driver code. Moreover by >>> avoiding hardcoding we can make it a generic driver for single bit PHYs. >>> >> >> +1. >> >> @Tomasz, any plans to consider this approach for simple phy driver? >> >> Regards, >> Rahul Sharma. >> > > Hi Rahul, > Initially, I wanted to make a very generic driver and to add bit and > register (or its offset) attribute to the PHY node. > However, there was a very strong opposition from DT maintainers > to adding any bit related configuration to DT. > The current solution was designed to be a trade-off between > being generic and being accepted :). > Thanks Tomasz, Ok got it. lets discuss it again and conclude it. @Kishon, DT-folks, The original RFC patch from Tomasz (at https://lkml.org/lkml/2013/10/21/313) added simple phy driver as "Generic-simple-phy" with these properties: + of_property_read_u32(dev->of_node, "mask", &sphy->mask); + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); Shall we consider the same solution again for generic simple phy driver which just expose on/off control through register bit. Regards, Rahul Sharma > Regards, > Tomasz Stanislawski > > > >>> Cheers >>> Kishon >> > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: allocate non-contigous buffers when iommu is enabled
From: Rahul Sharma
Allow to allocate non-contigous buffers when iommu is enabled.
Currently, it tries to allocates contigous buffer which consistently
fail for large buffers and then fall back to non contigous. Apart
from being slow, this implementation is also very noisy and fills
the screen with alloc fail logs.
Change-Id: I523e95aa308122ed2edc55e065ae6eb8be996541
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 5d88924..7136945 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -624,22 +624,20 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
args->pitch = args->width * ((args->bpp + 7) / 8);
args->size = args->pitch * args->height;
- exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG |
- EXYNOS_BO_WC, args->size);
- /*
-* If physically contiguous memory allocation fails and if IOMMU is
-* supported then try to get buffer from non physically contiguous
-* memory area.
-*/
- if (IS_ERR(exynos_gem_obj) && is_drm_iommu_supported(dev)) {
- dev_warn(dev->dev, "contiguous FB allocation failed, falling
back to non-contiguous\n");
+ if (is_drm_iommu_supported(dev)) {
+ exynos_gem_obj = exynos_drm_gem_create(dev,
+ EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
+ args->size);
+ } else {
exynos_gem_obj = exynos_drm_gem_create(dev,
- EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC,
- args->size);
+ EXYNOS_BO_CONTIG | EXYNOS_BO_WC,
+ args->size);
}
- if (IS_ERR(exynos_gem_obj))
+ if (IS_ERR(exynos_gem_obj)) {
+ dev_warn(dev->dev, "FB allocation failed.\n");
return PTR_ERR(exynos_gem_obj);
+ }
ret = exynos_drm_gem_handle_create(&exynos_gem_obj->base, file_priv,
&args->handle);
--
1.7.9.5
[PATCH] drm/exynos: use 4WORD dma burst length for small fbs
From: Rahul Sharma
In case of exynos, setting dma-burst to 16Word causes permanent
tearing for very small buffers, e.g. cursor buffer. Burst Mode
switching, which is based on overlay size is not recommended as
overlay size varies a lot towards the end of the screen. This
causes unstable DMA which results into tearing again.
Rendering small buffers with lower burst size doesn't
cause any noticable performance overhead. 128 pixel width is
selected based on mulitple experiments with exynos5 SoCs.
Signed-off-by: Rahul Sharma
Signed-off-by: Prathyush K
---
Based on Inki Dae's exynos-drm-next branch.
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 40fd6cc..ef56077 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -38,6 +38,7 @@
*/
#define FIMD_DEFAULT_FRAMERATE 60
+#define MIN_FB_WIDTH_FOR_16WORD_BURST 128
/* position control register for hardware window 0, 2 ~ 4.*/
#define VIDOSD_A(win) (VIDOSD_BASE + 0x00 + (win) * 16)
@@ -446,6 +447,19 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx,
unsigned int win)
DRM_DEBUG_KMS("bpp = %d\n", win_data->bpp);
+ /*
+* In case of exynos, setting dma-burst to 16Word causes permanent
+* tearing for very small buffers, e.g. cursor buffer. Burst Mode
+* switching which is based on overlay size is not recommended as
+* overlay size varies alot towards the end of the screen and rapid
+* movement causes unstable DMA which results into iommu crash/tear.
+*/
+
+ if (win_data->fb_width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
+ val &= ~WINCONx_BURSTLEN_MASK;
+ val |= WINCONx_BURSTLEN_4WORD;
+ }
+
writel(val, ctx->regs + WINCON(win));
}
--
1.7.9.5
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
On 5 May 2014 15:14, Kishon Vijay Abraham I wrote: > Hi, > > On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >> Hi, >> >> On 09/04/14 11:12, Rahul Sharma wrote: >>> Idea looks good. How about keeping compatible which is independent >>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>> and Bit through phy provider node. This way we can avoid SoC specific >>> hardcoding in phy driver and don't need to look into dt bindings for >>> each new SoC. >> >> I believe it is a not recommended approach. > > Why not? We should try to avoid hard coding in the driver code. Moreover by > avoiding hardcoding we can make it a generic driver for single bit PHYs. > +1. @Tomasz, any plans to consider this approach for simple phy driver? Regards, Rahul Sharma. > Cheers > Kishon
[PATCH v2 0/3] drm/exynos: enable support for exynos5420 hdmi
Hi Everybody, Gentle ping. Please have a look at the patches in this series? There are few more in pipeline. Please consider these as well. http://comments.gmane.org/gmane.linux.kernel.samsung-soc/30501 http://www.spinics.net/lists/linux-samsung-soc/msg28083.html Warm Regards, Rahul Sharma On 20 April 2014 15:53, Rahul Sharma wrote: > From: Rahul Sharma > > V2: > 1) Rebased on Tomasz Stanislawski's Simple phy driver patches > at https://lkml.org/lkml/2014/4/8/226. > > Adds apb mapped phy support for exynos5420 hdmi. Replace > dummy hdmiphy clock with regmap calls. > > Based on Inki Dae's exynos-drm-next branch. > > Rahul Sharma (3): > drm/exynos: remove unnecessary read for phy configuration values > drm/exynos: add support for apb mapped phys in hdmi driver > drm/exynos: enable support for exynos5420 hdmi device > > .../devicetree/bindings/video/exynos_hdmi.txt |1 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 317 > > drivers/gpu/drm/exynos/regs-hdmi.h |7 + > 3 files changed, 262 insertions(+), 63 deletions(-) > > -- > 1.7.9.5 >
[PATCH] drm/exynos: fix nested calls to lock mutex in drm resume
From: Rahul Sharma While testing S2R on exynos board, system is hanging and rebooting due to nested mutex_lock calls in exynos drm resume callback. Changing the order of the calls is fixing the issue. Signed-off-by: Rahul Sharma --- Based on exynos-drm-next branch in Inki Dae's tree. drivers/gpu/drm/exynos/exynos_drm_drv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index bb7dfee..2bb6233 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -184,8 +184,8 @@ static int exynos_drm_resume(struct drm_device *dev) connector->funcs->dpms(connector, connector->dpms); } - drm_helper_resume_force_mode(dev); drm_modeset_unlock_all(dev); + drm_helper_resume_force_mode(dev); return 0; } -- 1.7.9.5
[PATCH] drm/exynos: fix nested calls to lock mutex in drm resume
From: Rahul Sharma While testing S2R on exynos board, system is hanging and rebooting due to nested mutex_lock calls in exynos drm resume callback. Changing the order of the calls is fixing the issue. Change-Id: I3f3ada8a413a414dca0bbac53cfc5fe3138af4d6 Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_drm_drv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index bb7dfee..2bb6233 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -184,8 +184,8 @@ static int exynos_drm_resume(struct drm_device *dev) connector->funcs->dpms(connector, connector->dpms); } - drm_helper_resume_force_mode(dev); drm_modeset_unlock_all(dev); + drm_helper_resume_force_mode(dev); return 0; } -- 1.7.9.5
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
Sure (5250, 5420). I will wait for the same to update DT patches, if any.
Regards,
Rahul Sharma.
On 30 April 2014 14:02, Tomasz Stanislawski wrote:
> Hi Rahul,
> I will prepare we v3 version.
> Do you want me to add your patches for exynos5?50 to the patchset?
> Regards,
> Tomasz Stanislawski
>
> On 04/30/2014 08:37 AM, Rahul Sharma wrote:
>> Hi Tomasz,
>>
>> I have tested your patches for exynos5250 and 5420. Works fine. Are
>> you planning to post v3? If you want I can share hand with you for v3.
>>
>> Regards,
>> Rahul Sharma
>>
>> On 9 April 2014 17:17, Andreas Oberritter wrote:
>>> Hello Andrzej,
>>>
>>> On 09.04.2014 10:37, Andrzej Hajda wrote:
>>>>> +static int exynos_phy_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +const struct of_device_id *of_id = of_match_device(
>>>>> +of_match_ptr(exynos_phy_of_match), &pdev->dev);
>>>>> +const u32 *offsets = of_id->data;
>>>>> +int count;
>>>>> +struct device *dev = &pdev->dev;
>>>>> +struct phy **phys;
>>>>> +struct resource *res;
>>>>> +void __iomem *regs;
>>>>> +int i;
>>>>> +struct phy_provider *phy_provider;
>>>>> +
>>>>> +/* count number of phys to create */
>>>>> +for (count = 0; offsets[count] != ~0; ++count)
>>>>> +;
>>>>
>>>> count = ARRAY_SIZE(offsets) - 1;
>>>
>>> u32 *offsets is not an array.
>>>
>>> Regards,
>>> Andreas
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-samsung-soc" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
Hi Tomasz,
I have tested your patches for exynos5250 and 5420. Works fine. Are
you planning to post v3? If you want I can share hand with you for v3.
Regards,
Rahul Sharma
On 9 April 2014 17:17, Andreas Oberritter wrote:
> Hello Andrzej,
>
> On 09.04.2014 10:37, Andrzej Hajda wrote:
>>> +static int exynos_phy_probe(struct platform_device *pdev)
>>> +{
>>> +const struct of_device_id *of_id = of_match_device(
>>> +of_match_ptr(exynos_phy_of_match), &pdev->dev);
>>> +const u32 *offsets = of_id->data;
>>> +int count;
>>> +struct device *dev = &pdev->dev;
>>> +struct phy **phys;
>>> +struct resource *res;
>>> +void __iomem *regs;
>>> +int i;
>>> +struct phy_provider *phy_provider;
>>> +
>>> +/* count number of phys to create */
>>> +for (count = 0; offsets[count] != ~0; ++count)
>>> +;
>>
>> count = ARRAY_SIZE(offsets) - 1;
>
> u32 *offsets is not an array.
>
> Regards,
> Andreas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/exynos: enable fimd clocks in probe before accessing fimd registers
From: Rahul Sharma Fimd probe is accessing fimd Registers without enabling the fimd gate clocks. This hangs the system at boot time. This issue got surfaced when verifying with sysmmu enabled. Probe of fimd Sysmmu enables the master clock before accessing sysmmu regs and then disables. Later fimd probe tries to read the reegister without enabling the clock which is wrong and hangs the system. Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_drm_fimd.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 40fd6cc..8706fde 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -916,9 +916,15 @@ static int fimd_probe(struct platform_device *pdev) pm_runtime_enable(dev); + clk_prepare_enable(ctx->bus_clk); + clk_prepare_enable(ctx->lcd_clk); + for (win = 0; win < WINDOWS_NR; win++) fimd_clear_win(ctx, win); + clk_disable_unprepare(ctx->lcd_clk); + clk_disable_unprepare(ctx->bus_clk); + return 0; } -- 1.7.9.5
[PATCH v2 0/3] drm/exynos: enable support for exynos5420 hdmi
From: Rahul Sharma V2: 1) Rebased on Tomasz Stanislawski's Simple phy driver patches at https://lkml.org/lkml/2014/4/8/226. Adds apb mapped phy support for exynos5420 hdmi. Replace dummy hdmiphy clock with regmap calls. Based on Inki Dae's exynos-drm-next branch. Rahul Sharma (3): drm/exynos: remove unnecessary read for phy configuration values drm/exynos: add support for apb mapped phys in hdmi driver drm/exynos: enable support for exynos5420 hdmi device .../devicetree/bindings/video/exynos_hdmi.txt |1 + drivers/gpu/drm/exynos/exynos_hdmi.c | 317 drivers/gpu/drm/exynos/regs-hdmi.h |7 + 3 files changed, 262 insertions(+), 63 deletions(-) -- 1.7.9.5
[PATCH v2 3/3] drm/exynos: enable support for exynos5420 hdmi device
From: Rahul Sharma
Enable support for hdmi for exynos5420 hdmiphy. Add
compatible string in the of_match table. Also added
hdmiphy configuration values for exynos5420.
Signed-off-by: Rahul Sharma
Signed-off-by: Shirish S
---
.../devicetree/bindings/video/exynos_hdmi.txt |1 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 165
2 files changed, 166 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
index f9187a2..75ada04 100644
--- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
+++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
@@ -5,6 +5,7 @@ Required properties:
1) "samsung,exynos5-hdmi"
2) "samsung,exynos4210-hdmi"
3) "samsung,exynos4212-hdmi"
+ 4) "samsung,exynos5420-hdmi"
- reg: physical base address of the hdmi and length of memory mapped
region.
- interrupts: interrupt number to the cpu.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index f4566b1..93f48aa 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -425,6 +425,168 @@ static const struct hdmiphy_config hdmiphy_v14_configs[]
= {
},
};
+static const struct hdmiphy_config hdmiphy_5420_configs[] = {
+ {
+ .pixel_clock = 2520,
+ .conf = {
+ 0x01, 0x52, 0x3F, 0x55, 0x40, 0x01, 0x00, 0xC8,
+ 0x82, 0xC8, 0xBD, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x06, 0x80, 0x01, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xF4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 2700,
+ .conf = {
+ 0x01, 0xD1, 0x22, 0x51, 0x40, 0x08, 0xFC, 0xE0,
+ 0x98, 0xE8, 0xCB, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x06, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xE4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 27027000,
+ .conf = {
+ 0x01, 0xD1, 0x2D, 0x72, 0x40, 0x64, 0x12, 0xC8,
+ 0x43, 0xE8, 0x0E, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x26, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xE3, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 3600,
+ .conf = {
+ 0x01, 0x51, 0x2D, 0x55, 0x40, 0x40, 0x00, 0xC8,
+ 0x02, 0xC8, 0x0E, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xAB, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 4000,
+ .conf = {
+ 0x01, 0xD1, 0x21, 0x31, 0x40, 0x3C, 0x28, 0xC8,
+ 0x87, 0xE8, 0xC8, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0x9A, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 6500,
+ .conf = {
+ 0x01, 0xD1, 0x36, 0x34, 0x40, 0x0C, 0x04, 0xC8,
+ 0x82, 0xE8, 0x45, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xBD, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 7100,
+ .conf = {
+ 0x01, 0xD1, 0x3B, 0x35, 0x40, 0x0C, 0x04, 0xC8,
+ 0x85, 0xE8, 0x63, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0x57, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 7325,
+ .conf = {
+ 0x01, 0xD1, 0x1F, 0x10, 0x40, 0x78, 0x8D, 0xC8,
+ 0x81, 0xE8, 0xB7, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x56, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xA8, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 74176000,
+ .conf = {
+ 0x01, 0xD1, 0x1F, 0x10, 0x40, 0x5B, 0xEF, 0xC8,
+ 0x81, 0xE8, 0xB9, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x56, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xA6, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
[PATCH v2 2/3] drm/exynos: add support for apb mapped phys in hdmi driver
From: Rahul Sharma
Previous SoCs have hdmi phys which are accessible through
dedicated i2c lines. Newer SoCs have Apb mapped hdmi phys.
Hdmi driver is modified to support apb mapped phys.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 142 +-
drivers/gpu/drm/exynos/regs-hdmi.h |7 ++
2 files changed, 96 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 438eeda..f4566b1 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -67,6 +68,8 @@ enum hdmi_type {
struct hdmi_driver_data {
unsigned int type;
+ const struct hdmiphy_config *phy_confs;
+ unsigned int phy_conf_count;
unsigned int is_apb_phy:1;
};
@@ -196,6 +199,9 @@ struct hdmi_context {
struct hdmi_resources res;
int hpd_gpio;
+ void __iomem*regs_hdmiphy;
+ const struct hdmiphy_config *phy_confs;
+ unsigned intphy_conf_count;
enum hdmi_type type;
};
@@ -205,14 +211,6 @@ struct hdmiphy_config {
u8 conf[32];
};
-struct hdmi_driver_data exynos4212_hdmi_driver_data = {
- .type = HDMI_TYPE14,
-};
-
-struct hdmi_driver_data exynos5_hdmi_driver_data = {
- .type = HDMI_TYPE14,
-};
-
/* list of phy config settings */
static const struct hdmiphy_config hdmiphy_v13_configs[] = {
{
@@ -427,6 +425,21 @@ static const struct hdmiphy_config hdmiphy_v14_configs[] =
{
},
};
+
+struct hdmi_driver_data exynos4212_hdmi_driver_data = {
+ .type = HDMI_TYPE14,
+ .phy_confs = hdmiphy_v14_configs,
+ .phy_conf_count = ARRAY_SIZE(hdmiphy_v14_configs),
+ .is_apb_phy = 0,
+};
+
+struct hdmi_driver_data exynos5_hdmi_driver_data = {
+ .type = HDMI_TYPE14,
+ .phy_confs = hdmiphy_v13_configs,
+ .phy_conf_count = ARRAY_SIZE(hdmiphy_v13_configs),
+ .is_apb_phy = 0,
+};
+
static inline u32 hdmi_reg_read(struct hdmi_context *hdata, u32 reg_id)
{
return readl(hdata->regs + reg_id);
@@ -446,6 +459,48 @@ static inline void hdmi_reg_writemask(struct hdmi_context
*hdata,
writel(value, hdata->regs + reg_id);
}
+static int hdmiphy_reg_writeb(struct hdmi_context *hdata,
+ u32 reg_offset, u8 value)
+{
+ if (hdata->hdmiphy_port) {
+ u8 buffer[2];
+ int ret;
+
+ buffer[0] = reg_offset;
+ buffer[1] = value;
+
+ ret = i2c_master_send(hdata->hdmiphy_port, buffer, 2);
+ if (ret == 2)
+ return 0;
+ return ret;
+ } else {
+ writeb(value, hdata->regs_hdmiphy + (reg_offset<<2));
+ return 0;
+ }
+}
+
+static int hdmiphy_reg_write_buf(struct hdmi_context *hdata,
+ u32 reg_offset, const u8 *buf, u32 len)
+{
+ if ((reg_offset + len) > 32)
+ return -EINVAL;
+
+ if (hdata->hdmiphy_port) {
+ int ret;
+
+ ret = i2c_master_send(hdata->hdmiphy_port, buf, len);
+ if (ret == len)
+ return 0;
+ return ret;
+ } else {
+ int i;
+ for (i = 0; i < len; i++)
+ writeb(buf[i], hdata->regs_hdmiphy +
+ ((reg_offset + i)<<2));
+ return 0;
+ }
+}
+
static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix)
{
#define DUMPREG(reg_id) \
@@ -849,20 +904,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
static int hdmi_find_phy_conf(struct hdmi_context *hdata, u32 pixel_clock)
{
- const struct hdmiphy_config *confs;
- int count, i;
-
- if (hdata->type == HDMI_TYPE13) {
- confs = hdmiphy_v13_configs;
- count = ARRAY_SIZE(hdmiphy_v13_configs);
- } else if (hdata->type == HDMI_TYPE14) {
- confs = hdmiphy_v14_configs;
- count = ARRAY_SIZE(hdmiphy_v14_configs);
- } else
- return -EINVAL;
+ int i;
- for (i = 0; i < count; i++)
- if (confs[i].pixel_clock == pixel_clock)
+ for (i = 0; i < hdata->phy_conf_count; i++)
+ if (hdata->phy_confs[i].pixel_clock == pixel_clock)
return i;
DRM_DEBUG_KMS("Could not find phy config for %d\n", pixel_clock);
@@ -1514,17 +1559,9 @@ static void hdmiphy_poweroff(struct hdmi_context *hdata)
static void hdmiphy_conf_apply(struct hdmi_context *hdata)
{
- const u8 *hdmiphy_data;
- u8 buffer[32]
[PATCH v2 1/3] drm/exynos: remove unnecessary read for phy configuration values
From: Rahul Sharma
Cleaning up unnecessary i2c read call after hdmiphy configuration.
This check is redundant since check for hdmiphy pll lock status
confirms the correct settings for phy.
Signed-off-by: Rahul Sharma
Signed-off-by: Daniel Kurtz
Reviewed-by: Tomasz Figa
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index ef1cdd0..438eeda 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1517,7 +1517,6 @@ static void hdmiphy_conf_apply(struct hdmi_context *hdata)
const u8 *hdmiphy_data;
u8 buffer[32];
u8 operation[2];
- u8 read_buffer[32] = {0, };
int ret;
int i;
@@ -1557,15 +1556,6 @@ static void hdmiphy_conf_apply(struct hdmi_context
*hdata)
return;
}
- ret = i2c_master_recv(hdata->hdmiphy_port, read_buffer, 32);
- if (ret < 0) {
- DRM_ERROR("failed to read hdmiphy config\n");
- return;
- }
-
- for (i = 0; i < ret; i++)
- DRM_DEBUG_KMS("hdmiphy[0x%02x] write[0x%02x] - "
- "recv [0x%02x]\n", i, buffer[i], read_buffer[i]);
}
static void hdmi_conf_apply(struct hdmi_context *hdata)
--
1.7.9.5
[PATCH v2 0/3] drm/exynos: enable support for exynos5420 hdmi
From: Rahul Sharma V2: 1) Rebased on Tomasz Stanislawski at https://lkml.org/lkml/2014/4/8/226. Adds apb mapped phy support for exynos5420 hdmi. Replace dummy hdmiphy clock with regmap calls. Based on Inki Dae's exynos-drm-next branch. Rahul Sharma (3): drm/exynos: remove unnecessary read for phy configuration values drm/exynos: add support for apb mapped phys in hdmi driver drm/exynos: enable support for exynos5420 hdmi device .../devicetree/bindings/video/exynos_hdmi.txt |1 + drivers/gpu/drm/exynos/exynos_hdmi.c | 317 drivers/gpu/drm/exynos/regs-hdmi.h |7 + 3 files changed, 262 insertions(+), 63 deletions(-) -- 1.7.9.5
[PATCHv2 4/4] drm: exynos: hdmi: add support for pixel clock limitation
On 15 April 2014 19:59, Tomasz Stanislawski wrote:
> On 04/15/2014 03:42 PM, Rahul Sharma wrote:
>> On 15 April 2014 18:41, Tomasz Stanislawski
>> wrote:
>>> On 04/15/2014 11:42 AM, Rahul Sharma wrote:
>>>> Hi Tomasz,
>>>>
>>>> On 15 April 2014 14:57, Tomasz Stanislawski
>>>> wrote:
>>>>> Adds support for limitation of maximal pixel clock of HDMI
>>>>> signal. This feature is needed on boards that contains
>>>>> lines or bridges with frequency limitations.
>>>>>
>>>>> Signed-off-by: Tomasz Stanislawski
> [snip]
>
>>>>> diff --git a/include/media/s5p_hdmi.h b/include/media/s5p_hdmi.h
>>>>> index 181642b..7272d65 100644
>>>>> --- a/include/media/s5p_hdmi.h
>>>>> +++ b/include/media/s5p_hdmi.h
>>>>> @@ -31,6 +31,7 @@ struct s5p_hdmi_platform_data {
>>>>> int mhl_bus;
>>>>> struct i2c_board_info *mhl_info;
>>>>> int hpd_gpio;
>>>>> + u32 max_pixel_clock;
>>>>> };
>>>>
>>>> We have already removed Non DT support from the drm hdmi
>>>> driver. IMO we should not be extending the pdata struct.
>>>>
>>>> Regards,
>>>> Rahul Sharma
>>>
>>> Hi Rahul,
>>>
>>> This is not a non-DT patch. The s5p_hdmi_platform_data is
>>> generated from DT itself. This structure is just
>>> a parsed version of DT attributes.
>>>
>>> It may be a good idea to rename s5p_hdmi_platform_data
>>> to exynos_hdmi_pdata and move it to exynos_hdmi_drm.c file
>>> or parse DT directly in probe function.
>>>
>>> I can prepare a patch for that.
>>
>> Else we can completely remove the dependency from
>> s5p_hdmi_platform_data. We can directly assign to hdmi context
>> variables. Later we can remove that struct itself from include/.
>> What you say?
>
> This structure cannot be removed from include yet because it is used by
> s5p-tv driver.
> However its usage can be removed from both drivers.
> I can prepare both.
>
yea correct. but if you doing it for both of them, it can be removed, I guess.
your call.
Regards,
Rahul Sharma
>>
>> Regards,
>> Rahul Sharma
>>
>
> Regards,
> Tomasz Stanislawski
>
>>>
>>> Regards,
>>> Tomasz Stanislawski
>>>
>>>
>>>>
>>>>>
>>>>> #endif /* S5P_HDMI_H */
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>> ___
>>>>> dri-devel mailing list
>>>>> dri-devel at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>
[PATCHv2 4/4] drm: exynos: hdmi: add support for pixel clock limitation
On 15 April 2014 18:41, Tomasz Stanislawski wrote:
> On 04/15/2014 11:42 AM, Rahul Sharma wrote:
>> Hi Tomasz,
>>
>> On 15 April 2014 14:57, Tomasz Stanislawski
>> wrote:
>>> Adds support for limitation of maximal pixel clock of HDMI
>>> signal. This feature is needed on boards that contains
>>> lines or bridges with frequency limitations.
>>>
>>> Signed-off-by: Tomasz Stanislawski
>>> ---
>>> .../devicetree/bindings/video/exynos_hdmi.txt |4
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 12
>>> include/media/s5p_hdmi.h |1 +
>>> 3 files changed, 17 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> index f9187a2..8718f8d 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> @@ -28,6 +28,10 @@ Required properties:
>>> - ddc: phandle to the hdmi ddc node
>>> - phy: phandle to the hdmi phy node
>>>
>>> +Optional properties:
>>> +- max-pixel-clock: used to limit the maximal pixel clock if a board has
>>> lines,
>>> + connectors or bridges not capable of carring higher frequencies
>>> +
>>> Example:
>>>
>>> hdmi {
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index 2a18f4e..404f1b7 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -195,6 +195,7 @@ struct hdmi_context {
>>> struct hdmi_resources res;
>>>
>>> int hpd_gpio;
>>> + u32 max_pixel_clock;
>>>
>>> enum hdmi_type type;
>>> };
>>> @@ -887,6 +888,9 @@ static int hdmi_mode_valid(struct drm_connector
>>> *connector,
>>> if (ret)
>>> return MODE_BAD;
>>>
>>> + if (mode->clock * 1000 > hdata->max_pixel_clock)
>>> + return MODE_BAD;
>>> +
>>> ret = hdmi_find_phy_conf(hdata, mode->clock * 1000);
>>> if (ret < 0)
>>> return MODE_BAD;
>>> @@ -2031,6 +2035,8 @@ static struct s5p_hdmi_platform_data
>>> *drm_hdmi_dt_parse_pdata
>>> return NULL;
>>> }
>>>
>>> + of_property_read_u32(np, "max-pixel-clock", &pd->max_pixel_clock);
>>> +
>>> return pd;
>>> }
>>>
>>> @@ -2067,6 +2073,11 @@ static int hdmi_probe(struct platform_device *pdev)
>>> if (!pdata)
>>> return -EINVAL;
>>>
>>> + if (!pdata->max_pixel_clock) {
>>> + DRM_INFO("max-pixel-clock is zero, using INF\n");
>>> + pdata->max_pixel_clock = U32_MAX;
>>> + }
>>> +
>>> hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
>>> if (!hdata)
>>> return -ENOMEM;
>>> @@ -2083,6 +2094,7 @@ static int hdmi_probe(struct platform_device *pdev)
>>> hdata->type = drv_data->type;
>>>
>>> hdata->hpd_gpio = pdata->hpd_gpio;
>>> + hdata->max_pixel_clock = pdata->max_pixel_clock;
>>> hdata->dev = dev;
>>>
>>> ret = hdmi_resources_init(hdata);
>>> diff --git a/include/media/s5p_hdmi.h b/include/media/s5p_hdmi.h
>>> index 181642b..7272d65 100644
>>> --- a/include/media/s5p_hdmi.h
>>> +++ b/include/media/s5p_hdmi.h
>>> @@ -31,6 +31,7 @@ struct s5p_hdmi_platform_data {
>>> int mhl_bus;
>>> struct i2c_board_info *mhl_info;
>>> int hpd_gpio;
>>> + u32 max_pixel_clock;
>>> };
>>
>> We have already removed Non DT support from the drm hdmi
>> driver. IMO we should not be extending the pdata struct.
>>
>> Regards,
>> Rahul Sharma
>
> Hi Rahul,
>
> This is not a non-DT patch. The s5p_hdmi_platform_data is
> generated from DT itself. This structure is just
> a parsed version of DT attributes.
>
> It may be a good idea to rename s5p_hdmi_platform_data
> to exynos_hdmi_pdata and move it to exynos_hdmi_drm.c file
> or parse DT directly in probe function.
>
> I can prepare a patch for that.
Else we can completely remove the dependency from
s5p_hdmi_platform_data. We can directly assign to hdmi context
variables. Later we can remove that struct itself from include/.
What you say?
Regards,
Rahul Sharma
>
> Regards,
> Tomasz Stanislawski
>
>
>>
>>>
>>> #endif /* S5P_HDMI_H */
>>> --
>>> 1.7.9.5
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCHv2 4/4] drm: exynos: hdmi: add support for pixel clock limitation
Hi Tomasz,
On 15 April 2014 14:57, Tomasz Stanislawski wrote:
> Adds support for limitation of maximal pixel clock of HDMI
> signal. This feature is needed on boards that contains
> lines or bridges with frequency limitations.
>
> Signed-off-by: Tomasz Stanislawski
> ---
> .../devicetree/bindings/video/exynos_hdmi.txt |4
> drivers/gpu/drm/exynos/exynos_hdmi.c | 12
> include/media/s5p_hdmi.h |1 +
> 3 files changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> index f9187a2..8718f8d 100644
> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> @@ -28,6 +28,10 @@ Required properties:
> - ddc: phandle to the hdmi ddc node
> - phy: phandle to the hdmi phy node
>
> +Optional properties:
> +- max-pixel-clock: used to limit the maximal pixel clock if a board has
> lines,
> + connectors or bridges not capable of carring higher frequencies
> +
> Example:
>
> hdmi {
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2a18f4e..404f1b7 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -195,6 +195,7 @@ struct hdmi_context {
> struct hdmi_resources res;
>
> int hpd_gpio;
> + u32 max_pixel_clock;
>
> enum hdmi_type type;
> };
> @@ -887,6 +888,9 @@ static int hdmi_mode_valid(struct drm_connector
> *connector,
> if (ret)
> return MODE_BAD;
>
> + if (mode->clock * 1000 > hdata->max_pixel_clock)
> + return MODE_BAD;
> +
> ret = hdmi_find_phy_conf(hdata, mode->clock * 1000);
> if (ret < 0)
> return MODE_BAD;
> @@ -2031,6 +2035,8 @@ static struct s5p_hdmi_platform_data
> *drm_hdmi_dt_parse_pdata
> return NULL;
> }
>
> + of_property_read_u32(np, "max-pixel-clock", &pd->max_pixel_clock);
> +
> return pd;
> }
>
> @@ -2067,6 +2073,11 @@ static int hdmi_probe(struct platform_device *pdev)
> if (!pdata)
> return -EINVAL;
>
> + if (!pdata->max_pixel_clock) {
> + DRM_INFO("max-pixel-clock is zero, using INF\n");
> + pdata->max_pixel_clock = U32_MAX;
> + }
> +
> hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
> if (!hdata)
> return -ENOMEM;
> @@ -2083,6 +2094,7 @@ static int hdmi_probe(struct platform_device *pdev)
> hdata->type = drv_data->type;
>
> hdata->hpd_gpio = pdata->hpd_gpio;
> + hdata->max_pixel_clock = pdata->max_pixel_clock;
> hdata->dev = dev;
>
> ret = hdmi_resources_init(hdata);
> diff --git a/include/media/s5p_hdmi.h b/include/media/s5p_hdmi.h
> index 181642b..7272d65 100644
> --- a/include/media/s5p_hdmi.h
> +++ b/include/media/s5p_hdmi.h
> @@ -31,6 +31,7 @@ struct s5p_hdmi_platform_data {
> int mhl_bus;
> struct i2c_board_info *mhl_info;
> int hpd_gpio;
> + u32 max_pixel_clock;
> };
We have already removed Non DT support from the drm hdmi
driver. IMO we should not be extending the pdata struct.
Regards,
Rahul Sharma
>
> #endif /* S5P_HDMI_H */
> --
> 1.7.9.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 74121] New: [3.15-rc1] Exynos: Sandbox report fatal error "Unexpected 64bit argument detected"
-- Forwarded message -- From: Date: 15 April 2014 14:32 Subject: [Bug 74121] New: [3.15-rc1] Exynos: Sandbox report fatal error "Unexpected 64bit argument detected" To: dri-devel at lists.freedesktop.org https://bugzilla.kernel.org/show_bug.cgi?id=74121 Bug ID: 74121 Summary: [3.15-rc1] Exynos: Sandbox report fatal error "Unexpected 64bit argument detected" Product: Drivers Version: 2.5 Kernel Version: 3.15-rc1 Hardware: Other OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: r.sh.open at gmail.com Regression: No Sandbox crashing with 3.15-rc1 stating below fatal error. This was working in till 3.14 Stable. --- [1925:1925:0101/000305:FATAL:sandbox_bpf.cc(202)] Unexpected 64bit argument detected [1860:1860:1231/160305:FATAL:gpu_process_transport_factory.cc(210)] Failed to create UI context, but can't use software compositing with browser threaded compositing. Aborting. Handling SIGCHLD. Successfully wrote to child pipe. bootstrap_helper: ReserveBottomPages: NULL pointer guard page errno=22 --- [This is my first bug. Please let me know if things are not upto the mark.] -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel at lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/exynos: add support for apb mapped phys in hdmi driver
Thanks Tomasz,
On 10 April 2014 22:47, Tomasz Figa wrote:
> Hi Rahul,
>
> On 02.04.2014 19:13, Rahul Sharma wrote:
>>
>> From: Rahul Sharma
>>
>> Previous SoCs have hdmi phys which are accessible through
>> dedicated i2c lines. Newer SoCs have Apb mapped hdmi phys.
>> Hdmi driver is modified to support apb mapped phys.
>>
>> Signed-off-by: Rahul Sharma
>> ---
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 142
>> +-
>> drivers/gpu/drm/exynos/regs-hdmi.h |7 ++
>> 2 files changed, 96 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 5b2cfe7..5989770 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -33,6 +33,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -68,6 +69,8 @@ enum hdmi_type {
>>
>> struct hdmi_driver_data {
>> unsigned int type;
>> + const struct hdmiphy_config *phy_confs;
>> + unsigned int phy_conf_count;
>> unsigned int is_apb_phy:1;
>> };
>>
>> @@ -196,9 +199,12 @@ struct hdmi_context {
>> struct hdmi_resources res;
>>
>> int hpd_gpio;
>> + void __iomem*regs_hdmiphy;
>> struct regmap *pmureg;
>>
>> enum hdmi_type type;
>> + const struct hdmiphy_config *phy_confs;
>> + unsigned intphy_conf_count;
>> };
>>
>> struct hdmiphy_config {
>> @@ -206,14 +212,6 @@ struct hdmiphy_config {
>> u8 conf[32];
>> };
>>
>> -struct hdmi_driver_data exynos4212_hdmi_driver_data = {
>> - .type = HDMI_TYPE14,
>> -};
>> -
>> -struct hdmi_driver_data exynos5_hdmi_driver_data = {
>> - .type = HDMI_TYPE14,
>> -};
>> -
>> /* list of phy config settings */
>> static const struct hdmiphy_config hdmiphy_v13_configs[] = {
>> {
>> @@ -428,6 +426,21 @@ static const struct hdmiphy_config
>> hdmiphy_v14_configs[] = {
>> },
>> };
>>
>> +
>> +struct hdmi_driver_data exynos4212_hdmi_driver_data = {
>> + .type = HDMI_TYPE14,
>> + .phy_confs = hdmiphy_v14_configs,
>> + .phy_conf_count = ARRAY_SIZE(hdmiphy_v14_configs),
>> + .is_apb_phy = 0,
>> +};
>> +
>> +struct hdmi_driver_data exynos5_hdmi_driver_data = {
>> + .type = HDMI_TYPE14,
>> + .phy_confs = hdmiphy_v13_configs,
>> + .phy_conf_count = ARRAY_SIZE(hdmiphy_v13_configs),
>> + .is_apb_phy = 0,
>> +};
>> +
>> static inline u32 hdmi_reg_read(struct hdmi_context *hdata, u32 reg_id)
>> {
>> return readl(hdata->regs + reg_id);
>> @@ -447,6 +460,48 @@ static inline void hdmi_reg_writemask(struct
>> hdmi_context *hdata,
>> writel(value, hdata->regs + reg_id);
>> }
>>
>> +static int hdmiphy_reg_writeb(struct hdmi_context *hdata,
>> + u32 reg_offset, u8 value)
>> +{
>> + if (hdata->hdmiphy_port) {
>> + u8 buffer[2];
>> + int ret;
>> +
>> + buffer[0] = reg_offset;
>> + buffer[1] = value;
>> +
>> + ret = i2c_master_send(hdata->hdmiphy_port, buffer, 2);
>> + if (ret == 2)
>> + return 0;
>> + return ret;
>> + } else {
>> + writeb(value, hdata->regs_hdmiphy + (reg_offset<<2));
>> + return 0;
>> + }
>> +}
>> +
>> +static int hdmiphy_reg_write_buf(struct hdmi_context *hdata,
>> + u32 reg_offset, const u8 *buf, u32 len)
>> +{
>> + if ((reg_offset + len) > 32)
>> + return -EINVAL;
>> +
>> + if (hdata->hdmiphy_port) {
>> + int ret;
>> +
>> + ret = i2c_master_send(hdata->hdmiphy_port, buf, len);
>
>
> reg_offset doesn't seem to be used in I2C code path in any way. Are you sure
> this is correct?
>
yea ... reg_offset is not required for i2c write as first buffer in
buffer is taken as offset.
>> + if (ret == len)
>>
[PATCH 2/5] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
Thanks Tomasz,
This patch is not longer required after rebasing to Tomasz Stanislawski's
Simple Phy patches.
Regards,
Rahul Sharma.
On 10 April 2014 22:30, Tomasz Figa wrote:
> Hi Rahul,
>
> On 02.04.2014 19:13, Rahul Sharma wrote:
>>
>> From: Rahul Sharma
>>
>> Hdmiphy control bit needs to be set before setting the resolution
>> to hdmi hardware. This was handled using dummy hdmiphy clock which
>> is removed now.
>>
>> PMU is already defined as system controller for exynos SoC. Registers
>> of PMU are accessed using regmap interfaces.
>>
>> Devicetree binding document for hdmi is also updated.
>>
>> Signed-off-by: Rahul Sharma
>> ---
>> .../devicetree/bindings/video/exynos_hdmi.txt |2 ++
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 17
>> +
>> drivers/gpu/drm/exynos/regs-hdmi.h |4
>> 3 files changed, 23 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> index f9187a2..243a499 100644
>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> @@ -27,6 +27,7 @@ Required properties:
>> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
>> - ddc: phandle to the hdmi ddc node
>> - phy: phandle to the hdmi phy node
>> +- samsung,syscon-phandle: phandle for system controller node for PMU.
>>
>> Example:
>>
>> @@ -37,4 +38,5 @@ Example:
>> hpd-gpio = <&gpx3 7 1>;
>> ddc = <&hdmi_ddc_node>;
>> phy = <&hdmi_phy_node>;
>> + samsung,syscon-phandle = <&pmu_system_controller>;
>> };
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 23abfa0..47b8c06 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -36,6 +36,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include
>>
>> @@ -194,6 +196,7 @@ struct hdmi_context {
>> struct hdmi_resources res;
>>
>> int hpd_gpio;
>> + struct regmap *pmureg;
>>
>> enum hdmi_type type;
>> };
>> @@ -1853,6 +1856,9 @@ static void hdmi_poweron(struct exynos_drm_display
>> *display)
>> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>>
>> + /* set pmu hdmiphy control bit to enable hdmiphy */
>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> + PMU_HDMI_PHY_ENABLE_BIT, 1);
>> clk_prepare_enable(res->hdmi);
>> clk_prepare_enable(res->sclk_hdmi);
>>
>> @@ -1879,6 +1885,10 @@ static void hdmi_poweroff(struct exynos_drm_display
>> *display)
>>
>> clk_disable_unprepare(res->sclk_hdmi);
>> clk_disable_unprepare(res->hdmi);
>
>
> nit: Blank line would beautify the code a bit.
>
>> + /* reset pmu hdmiphy control bit to disable hdmiphy */
>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> + PMU_HDMI_PHY_ENABLE_BIT, 0);
>> +
>> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>>
>> pm_runtime_put_sync(hdata->dev);
>> @@ -2128,6 +2138,13 @@ static int hdmi_probe(struct platform_device *pdev)
>> goto err_hdmiphy;
>> }
>>
>> + hdata->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "samsung,syscon-phandle");
>> + if (IS_ERR_OR_NULL(hdata->pmureg)) {
>
>
> IS_ERR() is the correct macro to check return value of this function.
>
>> + DRM_ERROR("syscon regmap lookup failed.\n");
>> + goto err_hdmiphy;
>> + }
>> +
>> pm_runtime_enable(dev);
>>
>> hdmi_display.ctx = hdata;
>> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h
>> b/drivers/gpu/drm/exynos/regs-hdmi.h
>> index ef1b3eb..9811d6f 100644
>> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
>> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
>> @@ -578,4 +578,8 @@
>> #define HDMI_TG_VACT_ST4_HHDMI_TG_BASE(0x0074)
>> #define HDMI_TG_3DHDMI_TG_BASE(0x00F0)
>>
>> +/* PMU Registers for PHY */
>> +#define PMU_HDMI_PHY_CONTROL 0x700
>> +#define PMU_HDMI_PHY_ENABLE_BIT(1<<0)
>
>
> BIT() macro could be used instead of open coding the shift.
>
> Best regards,
> Tomasz
[PATCH 1/5] drm/exynos: remove dummy hdmiphy clock from hdmi driver
Hi Tomasz,
On 10 April 2014 21:02, Tomasz Figa wrote:
> Hi Rahul,
>
> On 02.04.2014 19:13, Rahul Sharma wrote:
>>
>> From: Rahul Sharma
>>
>> Exynos drm hdmi driver used to get dummy hdmiphy clock to
>> control the PMU bit for hdmiphy. This clock is removed
>> during CCF migration. This should also be cleaned from
>> hdmi driver.
>>
>> Signed-off-by: Rahul Sharma
>> ---
>> drivers/gpu/drm/exynos/exynos_hdmi.c |8
>> 1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 25bf65a..23abfa0 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -74,7 +74,6 @@ struct hdmi_resources {
>> struct clk *sclk_hdmi;
>> struct clk *sclk_pixel;
>> struct clk *sclk_hdmiphy;
>> - struct clk *hdmiphy;
>> struct clk *mout_hdmi;
>> struct regulator_bulk_data *regul_bulk;
>> int regul_count;
>> @@ -1854,7 +1853,6 @@ static void hdmi_poweron(struct exynos_drm_display
>> *display)
>> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>>
>> - clk_prepare_enable(res->hdmiphy);
>> clk_prepare_enable(res->hdmi);
>> clk_prepare_enable(res->sclk_hdmi);
>>
>> @@ -1881,7 +1879,6 @@ static void hdmi_poweroff(struct exynos_drm_display
>> *display)
>>
>> clk_disable_unprepare(res->sclk_hdmi);
>> clk_disable_unprepare(res->hdmi);
>> - clk_disable_unprepare(res->hdmiphy);
>> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>>
>> pm_runtime_put_sync(hdata->dev);
>> @@ -1977,11 +1974,6 @@ static int hdmi_resources_init(struct hdmi_context
>> *hdata)
>> DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
>> goto fail;
>> }
>> - res->hdmiphy = devm_clk_get(dev, "hdmiphy");
>> - if (IS_ERR(res->hdmiphy)) {
>> - DRM_ERROR("failed to get clock 'hdmiphy'\n");
>> - goto fail;
>> - }
>> res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
>> if (IS_ERR(res->mout_hdmi)) {
>> DRM_ERROR("failed to get clock 'mout_hdmi'\n");
>>
>
> This patch makes the series non-bisectable. If you remove handling of this
> dummy clock until you add proper support for PHY isolation setting, then at
> this point you end up with non-working code.
>
> You should first provide new infrastructure in parallel to existing one,
> then move all users to new one and only then drop the old one.
>
Actually, this is dead code since CCF migration. After this patch we will get
probe success but no UI. I will take care about bisection in my series.
Currently planning to discard first 2 patches and rebase on Tomasz
Stanislawski's
1 bit phy patches.
Regards,
Rahul Sharma
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 2/3] drm: exynos: hdmi: use hdmiphy as PHY
Hi Andrzej, On 9 April 2014 16:00, Andrzej Hajda wrote: > Hi Tomasz, > > On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote: >> The HDMIPHY (physical interface) is controlled by a single >> bit in a power controller's regiter. It was implemented >> as clock. It was a simple but effective hack. > > This power controller register has also bits to control HDMI clock > divider ratio. I guess current drivers do not change it, but how do you > want to implement access to it if some HDMI driver in the future will > need to change ratio. I guess in case of clk it would be easier. If it is really required to change this divider, it should be registered as a clock provider in clock driver exposing single divider clock. Regards, Rahul Sharma > > Regards > Andrzej > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 1/3] phy: Add exynos-simple-phy driver
t; +static struct phy_ops exynos_phy_ops = {
>> + .power_on = exynos_phy_power_on,
>> + .power_off = exynos_phy_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static const u32 exynos4210_offsets[] = {
>> + 0x0700, /* HDMI_PHY */
>> + 0x070C, /* DAC_PHY */
>> + 0x0718, /* ADC_PHY */
>> + 0x071C, /* PCIE_PHY */
>> + 0x0720, /* SATA_PHY */
>> + ~0, /* end mark */
>> +};
>> +
>> +static const u32 exynos4412_offsets[] = {
>> + 0x0700, /* HDMI_PHY */
>> + 0x0718, /* ADC_PHY */
>> + ~0, /* end mark */
>> +};
>
> Why have you selected only these registers?
> According to specs Exynos 4210 has 9 and 4412 has 7 control registers
> with 'phy-enable' functionality.
> I guess MIPI would require little more work as it has also reset bits,
> but it will be still better than separate driver.
>
>> +
>> +static const struct of_device_id exynos_phy_of_match[] = {
>> + { .compatible = "samsung,exynos4210-simple-phy",
>> + .data = exynos4210_offsets},
>> + { .compatible = "samsung,exynos4412-simple-phy",
>> + .data = exynos4412_offsets},
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_phy_of_match);
>> +
>> +static struct phy *exynos_phy_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct phy **phys = dev_get_drvdata(dev);
>> + int index = args->args[0];
>> + int i;
>> +
>> + /* verify if index is valid */
>> + for (i = 0; i <= index; ++i)
>> + if (!phys[i])
>> + return ERR_PTR(-ENODEV);
>> +
>> + return phys[index];
>> +}
>> +
>> +static int exynos_phy_probe(struct platform_device *pdev)
>> +{
>> + const struct of_device_id *of_id = of_match_device(
>> + of_match_ptr(exynos_phy_of_match), &pdev->dev);
>> + const u32 *offsets = of_id->data;
>> + int count;
>> + struct device *dev = &pdev->dev;
>> + struct phy **phys;
>> + struct resource *res;
>> + void __iomem *regs;
>> + int i;
>> + struct phy_provider *phy_provider;
>> +
>> + /* count number of phys to create */
>> + for (count = 0; offsets[count] != ~0; ++count)
>> + ;
>
> count = ARRAY_SIZE(offsets) - 1;
>
>> +
>> + phys = devm_kzalloc(dev, (count + 1) * sizeof(phys[0]), GFP_KERNEL);
>> + if (!phys)
>> + return -ENOMEM;
>> +
>> + dev_set_drvdata(dev, phys);
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> + regs = devm_ioremap(dev, res->start, res->end - res->start);
>> + if (!regs) {
>> + dev_err(dev, "failed to ioremap registers\n");
>> + return -EFAULT;
>> + }
>
> Why not devm_ioremap_resource? If not, resource_size function calculates
> length of resource correctly.
>
> Anyway I like the idea of implementing multiple phys in one driver.
> The only drawback I see is that some phys will be created even there are
> no consumers for them. To avoid such situation you can try to use
> lazy approach - create phy only if there is request for it,
> exynos_phy_xlate callback should allow this.
>
> Regards
> Andrzej
>
Idea looks good. How about keeping compatible which is independent
of SoC, something like "samsung,exynos-simple-phy" and provide Reg
and Bit through phy provider node. This way we can avoid SoC specific
hardcoding in phy driver and don't need to look into dt bindings for
each new SoC.
We can use syscon interface to access PMU bits like USB phy.
PMU is already registered as system controller
Regards,
Rahul Sharma.
>> +
>> + /* NOTE: last entry in phys[] is NULL */
>> + for (i = 0; i < count; ++i) {
>> + phys[i] = devm_phy_create(dev, &exynos_phy_ops, NULL);
>> + if (IS_ERR(phys[i])) {
>> + dev_err(dev, "failed to create PHY %d\n", i);
>> + return PTR_ERR(phys[i]);
>> + }
>> + phy_set_drvdata(phys[i], regs + offsets[i]);
>> + }
>> +
>> + phy_provider = devm_of_phy_provider_register(dev, exynos_phy_xlate);
>> + if (IS_ERR(phy_provider)) {
>> + dev_err(dev, "failed to register PHY provider\n");
>> + return PTR_ERR(phy_provider);
>> + }
>> +
>> + dev_info(dev, "added %d phys\n", count);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver exynos_phy_driver = {
>> + .probe = exynos_phy_probe,
>> + .driver = {
>> + .of_match_table = exynos_phy_of_match,
>> + .name = "exynos-simple-phy",
>> + .owner = THIS_MODULE,
>> + }
>> +};
>> +module_platform_driver(exynos_phy_driver);
>> +
>> +MODULE_DESCRIPTION("Exynos Simple PHY driver");
>> +MODULE_AUTHOR("Tomasz Stanislawski ");
>> +MODULE_LICENSE("GPL v2");
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
Thanks Inki,
On 3 April 2014 21:23, Inki Dae wrote:
> Hi Rahul,
>
> Thanks for contributions.
>
> 2014-04-03 2:13 GMT+09:00 Rahul Sharma :
>> From: Rahul Sharma
>>
>> Hdmiphy control bit needs to be set before setting the resolution
>> to hdmi hardware. This was handled using dummy hdmiphy clock which
>> is removed now.
>>
>> PMU is already defined as system controller for exynos SoC. Registers
>> of PMU are accessed using regmap interfaces.
>>
>> Devicetree binding document for hdmi is also updated.
>>
>> Signed-off-by: Rahul Sharma
>> ---
>> .../devicetree/bindings/video/exynos_hdmi.txt |2 ++
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 17 +
>> drivers/gpu/drm/exynos/regs-hdmi.h |4
>> 3 files changed, 23 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> index f9187a2..243a499 100644
>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>> @@ -27,6 +27,7 @@ Required properties:
>> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
>> - ddc: phandle to the hdmi ddc node
>> - phy: phandle to the hdmi phy node
>> +- samsung,syscon-phandle: phandle for system controller node for PMU.
>>
>> Example:
>>
>> @@ -37,4 +38,5 @@ Example:
>> hpd-gpio = <&gpx3 7 1>;
>> ddc = <&hdmi_ddc_node>;
>> phy = <&hdmi_phy_node>;
>> + samsung,syscon-phandle = <&pmu_system_controller>;
>
> System regiters could be controlled by phy framework, drivers/phy/*
> with 'phys' property so I think we would need to consider the phy
> framework. Especially, this patch adds a new property,
> samsung,syscon-phandle so I'm careful in merging - I'm not sure that
> we really need this property, and we couldn't really use existing phys
> property. So let's have more times for reviews.
>
I will do that. But still "syscon-phandle" property needs to be added
to hdmi phy bindings. Very similar to USB phys in
Documentation/devicetree/bindings/phy/samsung-phy.txt. I hope
that should be fine.
Please review my other patches also.
Regards,
Rahul Sharma
> Thanks,
> Inki Dae
>
>> };
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 23abfa0..47b8c06 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -36,6 +36,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include
>>
>> @@ -194,6 +196,7 @@ struct hdmi_context {
>> struct hdmi_resources res;
>>
>> int hpd_gpio;
>> + struct regmap *pmureg;
>>
>> enum hdmi_type type;
>> };
>> @@ -1853,6 +1856,9 @@ static void hdmi_poweron(struct exynos_drm_display
>> *display)
>> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>>
>> + /* set pmu hdmiphy control bit to enable hdmiphy */
>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> + PMU_HDMI_PHY_ENABLE_BIT, 1);
>> clk_prepare_enable(res->hdmi);
>> clk_prepare_enable(res->sclk_hdmi);
>>
>> @@ -1879,6 +1885,10 @@ static void hdmi_poweroff(struct exynos_drm_display
>> *display)
>>
>> clk_disable_unprepare(res->sclk_hdmi);
>> clk_disable_unprepare(res->hdmi);
>> + /* reset pmu hdmiphy control bit to disable hdmiphy */
>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> + PMU_HDMI_PHY_ENABLE_BIT, 0);
>> +
>> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>>
>> pm_runtime_put_sync(hdata->dev);
>> @@ -2128,6 +2138,13 @@ static int hdmi_probe(struct platform_device *pdev)
>> goto err_hdmiphy;
>> }
>>
>> + hdata->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "samsung,syscon-phandle");
>> + if (IS_ERR_OR_NULL(hdata->pm
[PATCH 7/7] drm/exynos: replace hdmi reset with hdmi disable
Before setting the core and timing generation registers,
hdmi driver resets the whole hdmi hardware, which also
resets the audio related registers.
Hdmi reset is replaced by hdmi disable which is called
just before setting the core and timing registers. It
also ensure that audio settings are not changed.
Signed-off-by: Rahul Sharma
Signed-off-by: Shirish S
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 40 ++
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index fce2f7b..4c69139 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -197,6 +197,7 @@ struct hdmi_context {
struct i2c_client *hdmiphy_port;
/* current hdmiphy conf regs */
+ struct drm_display_mode current_mode;
struct hdmi_conf_regs mode_conf;
struct hdmi_resources res;
@@ -1350,20 +1351,15 @@ static void hdmi_audio_control(struct hdmi_context
*hdata, bool onoff)
HDMI_ASP_EN : HDMI_ASP_DIS, HDMI_ASP_MASK);
}
-static void hdmi_conf_reset(struct hdmi_context *hdata)
+static void hdmi_start(struct hdmi_context *hdata, bool start)
{
- u32 reg;
+ u32 val = start ? HDMI_TG_EN : 0;
- if (hdata->type == HDMI_TYPE13)
- reg = HDMI_V13_CORE_RSTOUT;
- else
- reg = HDMI_CORE_RSTOUT;
+ if (hdata->current_mode.flags & DRM_MODE_FLAG_INTERLACE)
+ val |= HDMI_FIELD_EN;
- /* resetting HDMI core */
- hdmi_reg_writemask(hdata, reg, 0, HDMI_CORE_SW_RSTOUT);
- usleep_range(1, 12000);
- hdmi_reg_writemask(hdata, reg, ~0, HDMI_CORE_SW_RSTOUT);
- usleep_range(1, 12000);
+ hdmi_reg_writemask(hdata, HDMI_CON_0, val, HDMI_EN);
+ hdmi_reg_writemask(hdata, HDMI_TG_CMD, val, HDMI_TG_EN | HDMI_FIELD_EN);
}
static void hdmi_conf_init(struct hdmi_context *hdata)
@@ -1500,12 +1496,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context
*hdata)
clk_prepare_enable(hdata->res.sclk_hdmi);
/* enable HDMI and timing generator */
- hdmi_reg_writemask(hdata, HDMI_CON_0, ~0, HDMI_EN);
- if (core->int_pro_mode[0])
- hdmi_reg_writemask(hdata, HDMI_TG_CMD, ~0, HDMI_TG_EN |
- HDMI_FIELD_EN);
- else
- hdmi_reg_writemask(hdata, HDMI_TG_CMD, ~0, HDMI_TG_EN);
+ hdmi_start(hdata, true);
}
static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
@@ -1667,12 +1658,7 @@ static void hdmi_v14_mode_apply(struct hdmi_context
*hdata)
clk_prepare_enable(hdata->res.sclk_hdmi);
/* enable HDMI and timing generator */
- hdmi_reg_writemask(hdata, HDMI_CON_0, ~0, HDMI_EN);
- if (core->int_pro_mode[0])
- hdmi_reg_writemask(hdata, HDMI_TG_CMD, ~0, HDMI_TG_EN |
- HDMI_FIELD_EN);
- else
- hdmi_reg_writemask(hdata, HDMI_TG_CMD, ~0, HDMI_TG_EN);
+ hdmi_start(hdata, true);
}
static void hdmi_mode_apply(struct hdmi_context *hdata)
@@ -1788,7 +1774,7 @@ static void hdmi_conf_apply(struct hdmi_context *hdata)
hdmiphy_conf_apply(hdata);
mutex_lock(&hdata->hdmi_mutex);
- hdmi_conf_reset(hdata);
+ hdmi_start(hdata, false);
hdmi_conf_init(hdata);
mutex_unlock(&hdata->hdmi_mutex);
@@ -2029,6 +2015,9 @@ static void hdmi_mode_set(struct exynos_drm_display
*display,
m->vrefresh, (m->flags & DRM_MODE_FLAG_INTERLACE) ?
"INTERLACED" : "PROGERESSIVE");
+ /* preserve mode information for later use. */
+ drm_mode_copy(&hdata->current_mode, mode);
+
if (hdata->type == HDMI_TYPE13)
hdmi_v13_mode_set(hdata, mode);
else
@@ -2089,6 +2078,9 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
goto out;
mutex_unlock(&hdata->hdmi_mutex);
+ /* HDMI System Disable */
+ hdmi_reg_writemask(hdata, HDMI_CON_0, 0, HDMI_EN);
+
hdmiphy_poweroff(hdata);
cancel_delayed_work(&hdata->hotplug_work);
--
1.7.9.5
[PATCH 6/7] drm/exynos: Read hpd gpio in is_connected callback
From: Sean Paul
This patch adds a gpio read of hpd during the is_connected
callback. This fixes the case where hdmi is off going into
suspend and the cable is plugged in while suspended. In this
case, the hpd interrupt does not fire and is_connected will
return false.
Signed-off-by: Sean Paul
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index b2cbf43..fce2f7b 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1031,6 +1031,8 @@ static enum drm_connector_status hdmi_detect(struct
drm_connector *connector,
{
struct hdmi_context *hdata = ctx_from_connector(connector);
+ hdata->hpd = gpio_get_value(hdata->hpd_gpio);
+
return hdata->hpd ? connector_status_connected :
connector_status_disconnected;
}
--
1.7.9.5
[PATCH 5/7] drm/exynos: add hdmiphy power on/off sequence
From: Shirish S
This patch implements the power on/off sequence
of HDMI PHY in exynos5420 and exynos5250 as provided
by the hardware team.
This has been verified for mulitple iterations of
S2R.
Signed-off-by: Shirish S
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 40 +-
drivers/gpu/drm/exynos/regs-hdmi.h |7 +-
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 539e603..b2cbf43 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1711,16 +1711,44 @@ static void hdmiphy_conf_reset(struct hdmi_context
*hdata)
static void hdmiphy_poweron(struct hdmi_context *hdata)
{
- if (hdata->type == HDMI_TYPE14)
- hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0,
- HDMI_PHY_POWER_OFF_EN);
+ if (hdata->type != HDMI_TYPE14)
+ return;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_ENABLE_MODE_SET);
+ /* Phy Power On */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
+ HDMI_PHY_POWER_ON);
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_DISABLE_MODE_SET);
+ /* PHY SW Reset */
+ hdmiphy_conf_reset(hdata);
}
static void hdmiphy_poweroff(struct hdmi_context *hdata)
{
- if (hdata->type == HDMI_TYPE14)
- hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0,
- HDMI_PHY_POWER_OFF_EN);
+ if (hdata->type != HDMI_TYPE14)
+ return;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* PHY SW Reset */
+ hdmiphy_conf_reset(hdata);
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_ENABLE_MODE_SET);
+
+ /* PHY Power Off */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_POWER,
+ HDMI_PHY_POWER_OFF);
+
+ /* For PHY Mode Setting */
+ hdmiphy_reg_writeb(hdata, HDMIPHY_MODE_SET_DONE,
+ HDMI_PHY_DISABLE_MODE_SET);
}
static void hdmiphy_conf_apply(struct hdmi_context *hdata)
diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h
b/drivers/gpu/drm/exynos/regs-hdmi.h
index 344a5db..fd4c590 100644
--- a/drivers/gpu/drm/exynos/regs-hdmi.h
+++ b/drivers/gpu/drm/exynos/regs-hdmi.h
@@ -579,7 +579,12 @@
#define HDMI_TG_3D HDMI_TG_BASE(0x00F0)
/* HDMI PHY Registers Offsets*/
-#define HDMIPHY_MODE_SET_DONE (0x7C >> 2)
+#define HDMIPHY_POWER (0x74 >> 2)
+#define HDMIPHY_MODE_SET_DONE (0x7c >> 2)
+
+/* HDMI PHY Values */
+#define HDMI_PHY_POWER_ON 0x80
+#define HDMI_PHY_POWER_OFF 0xff
/* HDMI PHY Values */
#define HDMI_PHY_DISABLE_MODE_SET 0x80
--
1.7.9.5
[PATCH 4/7] drm/exynos: hdmi: remove unnecessary memset
From: Daniel Kurtz
Our resources were just zalloc'ed as part of hdata.
They are already 0.
Signed-off-by: Daniel Kurtz
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c |2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8973..539e603 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -2151,8 +2151,6 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
DRM_DEBUG_KMS("HDMI resource init\n");
- memset(res, 0, sizeof(*res));
-
/* get clocks, power */
res->hdmi = devm_clk_get(dev, "hdmi");
if (IS_ERR(res->hdmi)) {
--
1.7.9.5
[PATCH 3/7] drm/exynos: check for null pointers in error handling
From: Paul Taysom Smatch error from arm build: drivers/gpu/drm/exynos/ exynos_hdmi.c:2374 hdmi_probe() error: potential NULL dereference 'hdata->hdmiphy_port'. Added check for hdata->hdmiphy_port that it is not NULL. Signed-off-by: Paul Taysom Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_hdmi.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index f6d4435..5ed8973 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2356,7 +2356,8 @@ static int hdmi_probe(struct platform_device *pdev) return 0; err_hdmiphy: - put_device(&hdata->hdmiphy_port->dev); + if (hdata->hdmiphy_port) + put_device(&hdata->hdmiphy_port->dev); err_ddc: put_device(&hdata->ddc_adpt->dev); return ret; -- 1.7.9.5
[PATCH 2/7] drm/exynos: Debounce HDMI hotplug interrupts
From: Sean Paul
This patch debounces hotplug interrupts generated by the HDMI hotplug
gpio. The reason this is needed is that we get multiple (5) interrupts
every time a monitor is inserted which causes us to needlessly enable
and disable the IP block.
Signed-off-by: Sean Paul
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 79f98ac..f6d4435 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -51,6 +51,8 @@
#define get_hdmi_display(dev) platform_get_drvdata(to_platform_device(dev))
#define ctx_from_connector(c) container_of(c, struct hdmi_context, connector)
+#define HOTPLUG_DEBOUNCE_MS1100
+
/* AVI header and aspect ratio */
#define HDMI_AVI_VERSION 0x02
#define HDMI_AVI_LENGTH0x0D
@@ -189,6 +191,7 @@ struct hdmi_context {
void __iomem*regs;
int irq;
+ struct delayed_work hotplug_work;
struct i2c_adapter *ddc_adpt;
struct i2c_client *hdmiphy_port;
@@ -2058,6 +2061,8 @@ static void hdmi_poweroff(struct exynos_drm_display
*display)
hdmiphy_poweroff(hdata);
+ cancel_delayed_work(&hdata->hotplug_work);
+
clk_disable_unprepare(res->sclk_hdmi);
clk_disable_unprepare(res->hdmi);
/* reset pmu hdmiphy control bit to disable hdmiphy */
@@ -2108,9 +2113,11 @@ static struct exynos_drm_display hdmi_display = {
.ops = &hdmi_display_ops,
};
-static irqreturn_t hdmi_irq_thread(int irq, void *arg)
+static void hdmi_hotplug_work_func(struct work_struct *work)
{
- struct hdmi_context *hdata = arg;
+ struct hdmi_context *hdata;
+
+ hdata = container_of(work, struct hdmi_context, hotplug_work.work);
mutex_lock(&hdata->hdmi_mutex);
hdata->hpd = 1;//gpio_get_value(hdata->hpd_gpio);
@@ -2118,6 +2125,14 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg)
if (hdata->drm_dev)
drm_helper_hpd_irq_event(hdata->drm_dev);
+}
+
+static irqreturn_t hdmi_irq_thread(int irq, void *arg)
+{
+ struct hdmi_context *hdata = arg;
+
+ mod_delayed_work(system_wq, &hdata->hotplug_work,
+ msecs_to_jiffies(HOTPLUG_DEBOUNCE_MS));
return IRQ_HANDLED;
}
@@ -2315,6 +2330,8 @@ static int hdmi_probe(struct platform_device *pdev)
hdata->hpd = 1;//gpio_get_value(hdata->hpd_gpio);
+ INIT_DELAYED_WORK(&hdata->hotplug_work, hdmi_hotplug_work_func);
+
ret = devm_request_threaded_irq(dev, hdata->irq, NULL,
hdmi_irq_thread, IRQF_TRIGGER_RISING |
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
@@ -2351,6 +2368,8 @@ static int hdmi_remove(struct platform_device *pdev)
struct exynos_drm_display *display = get_hdmi_display(dev);
struct hdmi_context *hdata = display->ctx;
+ cancel_delayed_work_sync(&hdata->hotplug_work);
+
put_device(&hdata->hdmiphy_port->dev);
put_device(&hdata->ddc_adpt->dev);
pm_runtime_disable(&pdev->dev);
--
1.7.9.5
[PATCH 1/7] drm/exynos: Don't reset hdmiphy on hdmi off
From: Sean Paul This patch removes the hdmiphy reset in hdmi_poweroff. The hdmiphy reset was added to take advantage of exynos clockgating, doing it would gate the entire TV domain. Unfortunately, mixer is included in the TV domain and its vsync interrupts are stopped when TV is gated. Signed-off-by: Sean Paul Signed-off-by: Rahul Sharma --- drivers/gpu/drm/exynos/exynos_hdmi.c |5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index f3189af..79f98ac 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2056,11 +2056,6 @@ static void hdmi_poweroff(struct exynos_drm_display *display) goto out; mutex_unlock(&hdata->hdmi_mutex); - /* -* The TV power domain needs any condition of hdmiphy to turn off and -* its reset state seems to meet the condition. -*/ - hdmiphy_conf_reset(hdata); hdmiphy_poweroff(hdata); clk_disable_unprepare(res->sclk_hdmi); -- 1.7.9.5
[PATCH 0/7] drm/exynos: fixes for hdmi related issues
From: Rahul Sharma Series is addressing various issues in the drm hdmi driver for exynos Soc. Based on Inki Dae's exynos-drm-next branch. Daniel Kurtz (1): drm/exynos: hdmi: remove unnecessary memset Paul Taysom (1): drm/exynos: check for null pointers in error handling Rahul Sharma (1): drm/exynos: replace hdmi reset with hdmi disable Sean Paul (3): drm/exynos: Don't reset hdmiphy on hdmi off drm/exynos: Debounce HDMI hotplug interrupts drm/exynos: Read hpd gpio in is_connected callback Shirish S (1): drm/exynos: add hdmiphy power on/off sequence drivers/gpu/drm/exynos/exynos_hdmi.c | 115 ++ drivers/gpu/drm/exynos/regs-hdmi.h |7 ++- 2 files changed, 81 insertions(+), 41 deletions(-) -- 1.7.9.5
[PATCH 5/5] drm/exynos: enable support for exynos5420 hdmi device
From: Rahul Sharma
Enable support for hdmi for exynos5420 hdmiphy. Add
compatible string in the of_match table. Also added
hdmiphy configuration values for exynos5420.
Signed-off-by: Rahul Sharma
Signed-off-by: Shirish S
---
.../devicetree/bindings/video/exynos_hdmi.txt |1 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 165
2 files changed, 166 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
index 243a499..1fd8cf9 100644
--- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
+++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
@@ -5,6 +5,7 @@ Required properties:
1) "samsung,exynos5-hdmi"
2) "samsung,exynos4210-hdmi"
3) "samsung,exynos4212-hdmi"
+ 4) "samsung,exynos5420-hdmi"
- reg: physical base address of the hdmi and length of memory mapped
region.
- interrupts: interrupt number to the cpu.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5989770..f3189af 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -426,6 +426,168 @@ static const struct hdmiphy_config hdmiphy_v14_configs[]
= {
},
};
+static const struct hdmiphy_config hdmiphy_5420_configs[] = {
+ {
+ .pixel_clock = 2520,
+ .conf = {
+ 0x01, 0x52, 0x3F, 0x55, 0x40, 0x01, 0x00, 0xC8,
+ 0x82, 0xC8, 0xBD, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x06, 0x80, 0x01, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xF4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 2700,
+ .conf = {
+ 0x01, 0xD1, 0x22, 0x51, 0x40, 0x08, 0xFC, 0xE0,
+ 0x98, 0xE8, 0xCB, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x06, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xE4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 27027000,
+ .conf = {
+ 0x01, 0xD1, 0x2D, 0x72, 0x40, 0x64, 0x12, 0xC8,
+ 0x43, 0xE8, 0x0E, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x26, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xE3, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 3600,
+ .conf = {
+ 0x01, 0x51, 0x2D, 0x55, 0x40, 0x40, 0x00, 0xC8,
+ 0x02, 0xC8, 0x0E, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xAB, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 4000,
+ .conf = {
+ 0x01, 0xD1, 0x21, 0x31, 0x40, 0x3C, 0x28, 0xC8,
+ 0x87, 0xE8, 0xC8, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0x9A, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 6500,
+ .conf = {
+ 0x01, 0xD1, 0x36, 0x34, 0x40, 0x0C, 0x04, 0xC8,
+ 0x82, 0xE8, 0x45, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xBD, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 7100,
+ .conf = {
+ 0x01, 0xD1, 0x3B, 0x35, 0x40, 0x0C, 0x04, 0xC8,
+ 0x85, 0xE8, 0x63, 0xD9, 0x45, 0xA0, 0xAC, 0x80,
+ 0x08, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0x57, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 7325,
+ .conf = {
+ 0x01, 0xD1, 0x1F, 0x10, 0x40, 0x78, 0x8D, 0xC8,
+ 0x81, 0xE8, 0xB7, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x56, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xA8, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
+ .pixel_clock = 74176000,
+ .conf = {
+ 0x01, 0xD1, 0x1F, 0x10, 0x40, 0x5B, 0xEF, 0xC8,
+ 0x81, 0xE8, 0xB9, 0xD8, 0x45, 0xA0, 0xAC, 0x80,
+ 0x56, 0x80, 0x09, 0x84, 0x05, 0x02, 0x24, 0x66,
+ 0x54, 0xA6, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80,
+ },
+ },
+ {
