| Message ID | 1425906667-3363-6-git-send-email-sviau@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi, On 03/09/2015 06:41 PM, Stephane Viau wrote: > This change adds the hw configuration for msm8x16 chipsets in > mdp5_cfg module. > > Note that only one external display interface is present in this > configuration (DSI) but has not been enabled yet. It will be enabled > once drm/msm driver supports DSI connectors. > > Signed-off-by: Stephane Viau <sviau@codeaurora.org> > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 ++++++++++++++++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c > index 96ea6dd..9ff7ac1 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2014 The Linux Foundation. All rights reserved. > + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 and > @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = { > .max_clk = 320000000, > }; > > +const struct mdp5_cfg_hw msm8x16_config = { > + .name = "msm8x16", > + .mdp = { > + .count = 1, > + .base = { 0x01000 }, > + }, > + .smp = { > + .mmb_count = 8, > + .mmb_size = 8192, > + .clients = { > + [SSPP_VIG0] = 1, [SSPP_DMA0] = 4, > + [SSPP_RGB0] = 7, [SSPP_RGB1] = 8, > + }, > + }, > + .ctl = { > + .count = 5, > + .base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 }, > + }, > + .pipe_vig = { > + .count = 1, > + .base = { 0x05000 }, > + }, > + .pipe_rgb = { > + .count = 2, > + .base = { 0x15000, 0x17000 }, > + }, > + .pipe_dma = { > + .count = 1, > + .base = { 0x25000 }, > + }, > + .lm = { > + .count = 2, /* LM0 and LM3 */ > + .base = { 0x45000, 0x48000 }, > + .nb_stages = 5, > + }, > + .dspp = { > + .count = 1, > + .base = { 0x55000 }, > + > + }, > + .intf = { > + .count = 1, /* INTF_1 */ > + .base = { 0x6B800 }, We would need to put the other non-existent INTF_0, INTF_2 and INTF_3 base addresses here too, so that the writes to REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in config module" access the correct registers. Archit > + }, > + /* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented */ > + .max_clk = 320000000, > +}; > + > static const struct mdp5_cfg_handler cfg_handlers[] = { > { .revision = 0, .config = { .hw = &msm8x74_config } }, > { .revision = 2, .config = { .hw = &msm8x74_config } }, > { .revision = 3, .config = { .hw = &apq8084_config } }, > + { .revision = 6, .config = { .hw = &msm8x16_config } }, > }; > > >
Hi, > Hi, > > On 03/09/2015 06:41 PM, Stephane Viau wrote: >> This change adds the hw configuration for msm8x16 chipsets in >> mdp5_cfg module. >> >> Note that only one external display interface is present in this >> configuration (DSI) but has not been enabled yet. It will be enabled >> once drm/msm driver supports DSI connectors. >> >> Signed-off-by: Stephane Viau <sviau@codeaurora.org> >> --- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 >> ++++++++++++++++++++++++++++++++- >> 1 file changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >> index 96ea6dd..9ff7ac1 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (c) 2014 The Linux Foundation. All rights reserved. >> + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved. >> * >> * This program is free software; you can redistribute it and/or >> modify >> * it under the terms of the GNU General Public License version 2 and >> @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = { >> .max_clk = 320000000, >> }; >> >> +const struct mdp5_cfg_hw msm8x16_config = { >> + .name = "msm8x16", >> + .mdp = { >> + .count = 1, >> + .base = { 0x01000 }, >> + }, >> + .smp = { >> + .mmb_count = 8, >> + .mmb_size = 8192, >> + .clients = { >> + [SSPP_VIG0] = 1, [SSPP_DMA0] = 4, >> + [SSPP_RGB0] = 7, [SSPP_RGB1] = 8, >> + }, >> + }, >> + .ctl = { >> + .count = 5, >> + .base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 }, >> + }, >> + .pipe_vig = { >> + .count = 1, >> + .base = { 0x05000 }, >> + }, >> + .pipe_rgb = { >> + .count = 2, >> + .base = { 0x15000, 0x17000 }, >> + }, >> + .pipe_dma = { >> + .count = 1, >> + .base = { 0x25000 }, >> + }, >> + .lm = { >> + .count = 2, /* LM0 and LM3 */ >> + .base = { 0x45000, 0x48000 }, >> + .nb_stages = 5, >> + }, >> + .dspp = { >> + .count = 1, >> + .base = { 0x55000 }, >> + >> + }, >> + .intf = { >> + .count = 1, /* INTF_1 */ >> + .base = { 0x6B800 }, > > We would need to put the other non-existent INTF_0, INTF_2 and INTF_3 > base addresses here too, so that the writes to > REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in > config module" access the correct registers. You are referring here to the discussion we had in "drm/msm/mdp5: Make the intf connection in config module"[1]... Let me clarify: We see these faults when interfaces are *present* but not *supported* by the driver, where: - *present* means that the interfaces are physically implemented in HW and therefore need to be reflected by "intf.base" addresses - *supported* means that the msm KMS driver is actually able to drive an interface If you look at mdp5_cfg.c in "drm/msm/mdp5: Make the intf connection in config module"[1], 4 interfaces (intf.base) are present (for apq8084), but only 2 are supported (intfs[])... What I meant is that even though our driver cannot support all interfaces present in a chip, we still need to disable them in mdp5_kms_init(). (BTW, this is probably due to my bootloader enabling the DSI whereas the msm KMS driver does not support it as of yet - and thus leads to artifacts/underflow on my eDP panel.) So to answer your comment, it doesn't make too much sense to define base addresses for interfaces that are not present in a chip. Instead I'd prefer to check if intf.base is not NULL before writing in REG_MDP5_INTF_TIMING_ENGINE_EN registers.. and avoid some INVALID_INDEX BUGs ;-). I'm sending out a v3 for "drm/msm/mdp5: Make the intf connection in config module" in a minute... [1] http://lists.freedesktop.org/archives/dri-devel/2015-March/078651.html > > Archit > >> + }, >> + /* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented >> */ >> + .max_clk = 320000000, >> +}; >> + >> static const struct mdp5_cfg_handler cfg_handlers[] = { >> { .revision = 0, .config = { .hw = &msm8x74_config } }, >> { .revision = 2, .config = { .hw = &msm8x74_config } }, >> { .revision = 3, .config = { .hw = &apq8084_config } }, >> + { .revision = 6, .config = { .hw = &msm8x16_config } }, >> }; >> >> >> > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
WB and DSI support are in the pipe and will come out soon. Before that, we need to prepare the MDP5 driver so we can support these connectors. v2: rename macro to mdp5_cfg_intf_is_virtual() [pointed by Archit] v3: add sanity check before writing in INTF_TIMING_ENGINE_EN registers Note: "drm/msm: Add display configuration for msm8x16" patch set depends on "drm/msm: preparation for WB/DSI connectors" patch set. Stephane Viau (4): drm/msm/mdp5: Update generated header files drm/msm/mdp5: Enhance operation mode for pipeline configuration drm/msm/mdp5: Add START signal to kick off certain pipelines drm/msm/mdp5: Make the intf connection in config module drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h | 68 +++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 10 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 15 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 70 ++---- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 326 +++++++++++++++++++++++++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h | 75 +++---- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 40 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 100 +++++---- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 49 ++++- 9 files changed, 522 insertions(+), 231 deletions(-)
On 03/14/2015 01:15 AM, "Stéphane Viau" wrote: > Hi, > >> Hi, >> >> On 03/09/2015 06:41 PM, Stephane Viau wrote: >>> This change adds the hw configuration for msm8x16 chipsets in >>> mdp5_cfg module. >>> >>> Note that only one external display interface is present in this >>> configuration (DSI) but has not been enabled yet. It will be enabled >>> once drm/msm driver supports DSI connectors. >>> >>> Signed-off-by: Stephane Viau <sviau@codeaurora.org> >>> --- >>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 >>> ++++++++++++++++++++++++++++++++- >>> 1 file changed, 50 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >>> index 96ea6dd..9ff7ac1 100644 >>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c >>> @@ -1,5 +1,5 @@ >>> /* >>> - * Copyright (c) 2014 The Linux Foundation. All rights reserved. >>> + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved. >>> * >>> * This program is free software; you can redistribute it and/or >>> modify >>> * it under the terms of the GNU General Public License version 2 and >>> @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = { >>> .max_clk = 320000000, >>> }; >>> >>> +const struct mdp5_cfg_hw msm8x16_config = { >>> + .name = "msm8x16", >>> + .mdp = { >>> + .count = 1, >>> + .base = { 0x01000 }, >>> + }, >>> + .smp = { >>> + .mmb_count = 8, >>> + .mmb_size = 8192, >>> + .clients = { >>> + [SSPP_VIG0] = 1, [SSPP_DMA0] = 4, >>> + [SSPP_RGB0] = 7, [SSPP_RGB1] = 8, >>> + }, >>> + }, >>> + .ctl = { >>> + .count = 5, >>> + .base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 }, >>> + }, >>> + .pipe_vig = { >>> + .count = 1, >>> + .base = { 0x05000 }, >>> + }, >>> + .pipe_rgb = { >>> + .count = 2, >>> + .base = { 0x15000, 0x17000 }, >>> + }, >>> + .pipe_dma = { >>> + .count = 1, >>> + .base = { 0x25000 }, >>> + }, >>> + .lm = { >>> + .count = 2, /* LM0 and LM3 */ >>> + .base = { 0x45000, 0x48000 }, >>> + .nb_stages = 5, >>> + }, >>> + .dspp = { >>> + .count = 1, >>> + .base = { 0x55000 }, >>> + >>> + }, >>> + .intf = { >>> + .count = 1, /* INTF_1 */ >>> + .base = { 0x6B800 }, >> >> We would need to put the other non-existent INTF_0, INTF_2 and INTF_3 >> base addresses here too, so that the writes to >> REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in >> config module" access the correct registers. > > You are referring here to the discussion we had in "drm/msm/mdp5: Make the > intf connection in config module"[1]... > > Let me clarify: > We see these faults when interfaces are *present* but not *supported* by > the driver, where: > - *present* means that the interfaces are physically implemented in HW > and therefore need to be reflected by "intf.base" addresses > - *supported* means that the msm KMS driver is actually able to drive an > interface > If you look at mdp5_cfg.c in "drm/msm/mdp5: Make the intf connection in > config module"[1], 4 interfaces (intf.base) are present (for apq8084), but > only 2 are supported (intfs[])... > What I meant is that even though our driver cannot support all interfaces > present in a chip, we still need to disable them in mdp5_kms_init(). > (BTW, this is probably due to my bootloader enabling the DSI whereas the > msm KMS driver does not support it as of yet - and thus leads to > artifacts/underflow on my eDP panel.) Okay, I get it now. I think I wasn't sure about whether INTF_DISABLED represents *present* or *supported*. Thanks for the clarification. > > So to answer your comment, it doesn't make too much sense to define base > addresses for interfaces that are not present in a chip. Instead I'd > prefer to check if intf.base is not NULL before writing in > REG_MDP5_INTF_TIMING_ENGINE_EN registers.. and avoid some INVALID_INDEX > BUGs ;-). That sounds good. Thanks. Archit
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c index 96ea6dd..9ff7ac1 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = { .max_clk = 320000000, }; +const struct mdp5_cfg_hw msm8x16_config = { + .name = "msm8x16", + .mdp = { + .count = 1, + .base = { 0x01000 }, + }, + .smp = { + .mmb_count = 8, + .mmb_size = 8192, + .clients = { + [SSPP_VIG0] = 1, [SSPP_DMA0] = 4, + [SSPP_RGB0] = 7, [SSPP_RGB1] = 8, + }, + }, + .ctl = { + .count = 5, + .base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 }, + }, + .pipe_vig = { + .count = 1, + .base = { 0x05000 }, + }, + .pipe_rgb = { + .count = 2, + .base = { 0x15000, 0x17000 }, + }, + .pipe_dma = { + .count = 1, + .base = { 0x25000 }, + }, + .lm = { + .count = 2, /* LM0 and LM3 */ + .base = { 0x45000, 0x48000 }, + .nb_stages = 5, + }, + .dspp = { + .count = 1, + .base = { 0x55000 }, + + }, + .intf = { + .count = 1, /* INTF_1 */ + .base = { 0x6B800 }, + }, + /* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented */ + .max_clk = 320000000, +}; + static const struct mdp5_cfg_handler cfg_handlers[] = { { .revision = 0, .config = { .hw = &msm8x74_config } }, { .revision = 2, .config = { .hw = &msm8x74_config } }, { .revision = 3, .config = { .hw = &apq8084_config } }, + { .revision = 6, .config = { .hw = &msm8x16_config } }, };
This change adds the hw configuration for msm8x16 chipsets in mdp5_cfg module. Note that only one external display interface is present in this configuration (DSI) but has not been enabled yet. It will be enabled once drm/msm driver supports DSI connectors. Signed-off-by: Stephane Viau <sviau@codeaurora.org> --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 ++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)