| Message ID | 20221119173019.15643-4-a-bhatia1@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DSS support for AM625 SoC | expand |
Hi, On 19/11/2022 19:30, Aradhya Bhatia wrote: > The newer version of DSS (AM625-DSS) has 2 OLDI TXes at its disposal. > These can be configured to support the following modes: > > 1. OLDI_SINGLE_LINK_SINGLE_MODE > Single Output over OLDI 0. > +------+ +---------+ +-------+ > | | | | | | > | CRTC +------->+ ENCODER +----->| PANEL | > | | | | | | > +------+ +---------+ +-------+ > > 2. OLDI_SINGLE_LINK_CLONE_MODE > Duplicate Output over OLDI 0 and 1. > +------+ +---------+ +-------+ > | | | | | | > | CRTC +---+--->| ENCODER +----->| PANEL | > | | | | | | | > +------+ | +---------+ +-------+ > | > | +---------+ +-------+ > | | | | | > +--->| ENCODER +----->| PANEL | > | | | | > +---------+ +-------+ > > 3. OLDI_DUAL_LINK_MODE > Combined Output over OLDI 0 and 1. > +------+ +---------+ +-------+ > | | | +----->| | > | CRTC +------->+ ENCODER | | PANEL | > | | | +----->| | > +------+ +---------+ +-------+ > > Following the above pathways for different modes, 2 encoder/panel-bridge > pipes get created for clone mode, and 1 pipe in cases of single link and > dual link mode. > > Add support for confguring the OLDI modes using OF and LVDS DRM helper > functions. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > drivers/gpu/drm/tidss/tidss_dispc.c | 12 ++ > drivers/gpu/drm/tidss/tidss_dispc.h | 9 ++ > drivers/gpu/drm/tidss/tidss_drv.h | 3 + > drivers/gpu/drm/tidss/tidss_encoder.c | 4 +- > drivers/gpu/drm/tidss/tidss_encoder.h | 3 +- > drivers/gpu/drm/tidss/tidss_kms.c | 188 +++++++++++++++++++++++--- > 6 files changed, 198 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c > index dbc6a5b617ca..472226a83251 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.c > +++ b/drivers/gpu/drm/tidss/tidss_dispc.c > @@ -365,6 +365,8 @@ struct dispc_device { > > struct dss_vp_data vp_data[TIDSS_MAX_VPS]; > > + enum dispc_oldi_modes oldi_mode; > + > u32 *fourccs; > u32 num_fourccs; > > @@ -1967,6 +1969,16 @@ const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len) > return dispc->fourccs; > } > > +int dispc_set_oldi_mode(struct dispc_device *dispc, > + enum dispc_oldi_modes oldi_mode) > +{ > + WARN_ON(!dispc); This feels unnecessary. Is there even a theoretical case where we could get dispc == NULL? > + > + dispc->oldi_mode = oldi_mode; > + > + return 0; This function could as well be void function. > +} > + > static s32 pixinc(int pixels, u8 ps) > { > if (pixels == 1) > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h > index 51db500590ee..e76a7599b544 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.h > +++ b/drivers/gpu/drm/tidss/tidss_dispc.h > @@ -64,6 +64,14 @@ enum dispc_dss_subrevision { > DISPC_AM625, > }; > > +enum dispc_oldi_modes { > + OLDI_MODE_OFF, /* OLDI turned off / tied off in IP. */ > + OLDI_SINGLE_LINK_SINGLE_MODE, /* Single Output over OLDI 0. */ > + OLDI_SINGLE_LINK_CLONE_MODE, /* Duplicate Output over OLDI 0 and 1. */ > + OLDI_DUAL_LINK_MODE, /* Combined Output over OLDI 0 and 1. */ > + OLDI_MODE_UNSUPPORTED, /* Unsupported OLDI Mode */ > +}; What is the difference with MODE_OFF and MODE_UNSUPPORTED? Is MODE_UNSUPPORTED for cases where, e.g., the DT setup is wrong and the driver should return an error? The code doesn't quite do that, it prints an error but then continues. > struct dispc_features { > int min_pclk_khz; > int max_pclk_khz[DISPC_VP_MAX_BUS_TYPE]; > @@ -133,6 +141,7 @@ int dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane, > u32 hw_videoport); > int dispc_plane_enable(struct dispc_device *dispc, u32 hw_plane, bool enable); > const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len); > +int dispc_set_oldi_mode(struct dispc_device *dispc, enum dispc_oldi_modes oldi_mode); > > int dispc_init(struct tidss_device *tidss); > void dispc_remove(struct tidss_device *tidss); > diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h > index 0ce7ee5ccd5b..58892f065c16 100644 > --- a/drivers/gpu/drm/tidss/tidss_drv.h > +++ b/drivers/gpu/drm/tidss/tidss_drv.h > @@ -13,6 +13,9 @@ > #define TIDSS_MAX_PLANES 4 > #define TIDSS_MAX_OUTPUT_PORTS 4 > > +/* For AM625-DSS with 2 OLDI TXes */ > +#define TIDSS_MAX_BRIDGES_PER_PIPE 2 > + > typedef u32 dispc_irq_t; > > struct tidss_device { > diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c > index e278a9c89476..141383ec4045 100644 > --- a/drivers/gpu/drm/tidss/tidss_encoder.c > +++ b/drivers/gpu/drm/tidss/tidss_encoder.c > @@ -70,7 +70,8 @@ static const struct drm_encoder_funcs encoder_funcs = { > }; > > struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, > - u32 encoder_type, u32 possible_crtcs) > + u32 encoder_type, u32 possible_crtcs, > + u32 possible_clones) > { > struct drm_encoder *enc; > int ret; > @@ -80,6 +81,7 @@ struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, > return ERR_PTR(-ENOMEM); > > enc->possible_crtcs = possible_crtcs; > + enc->possible_clones = possible_clones; > > ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs, > encoder_type, NULL); > diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h > index ace877c0e0fd..01c62ba3ef16 100644 > --- a/drivers/gpu/drm/tidss/tidss_encoder.h > +++ b/drivers/gpu/drm/tidss/tidss_encoder.h > @@ -12,6 +12,7 @@ > struct tidss_device; > > struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, > - u32 encoder_type, u32 possible_crtcs); > + u32 encoder_type, u32 possible_crtcs, > + u32 possible_clones); > > #endif > diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c > index fc9c4eefd31d..8ae321f02197 100644 > --- a/drivers/gpu/drm/tidss/tidss_kms.c > +++ b/drivers/gpu/drm/tidss/tidss_kms.c > @@ -106,30 +106,115 @@ static const struct drm_mode_config_funcs mode_config_funcs = { > .atomic_commit = drm_atomic_helper_commit, > }; > > +static enum dispc_oldi_modes tidss_get_oldi_mode(struct device_node *oldi0_port, > + struct device_node *oldi1_port) > +{ > + int pixel_order; > + > + if (!(oldi0_port || oldi1_port)) { > + /* Keep OLDI TXes off if neither OLDI port is present. */ > + return OLDI_MODE_OFF; > + } else if (oldi0_port && !oldi1_port) { > + /* > + * OLDI0 port found, but not OLDI1 port. Setting single > + * link, single mode output. > + */ > + return OLDI_SINGLE_LINK_SINGLE_MODE; > + } else if (!oldi0_port && oldi1_port) { > + /* > + * The 2nd OLDI TX cannot be operated alone. This use case is > + * not supported in the HW. Since the pins for OLDIs 0 and 1 are > + * separate, one could theoretically set a clone mode over OLDIs > + * 0 and 1 and just simply not use the OLDI 0. This is a hacky > + * way to enable only OLDI TX 1 and hence is not officially > + * supported. > + */ > + return OLDI_MODE_UNSUPPORTED; If OLDI_MODE_UNSUPPORTED is supposed to result in an error, maybe you could print the error here (and possibly in the default case below), and then, in the caller, just return with an error code. > + } > + > + /* > + * OLDI Ports found for both the OLDI TXes. The DSS is to be configured > + * in either Dual Link or Clone Mode. > + */ > + pixel_order = drm_of_lvds_get_dual_link_pixel_order(oldi0_port, > + oldi1_port); > + switch (pixel_order) { > + case -EINVAL: > + /* > + * The dual link properties were not found in at least one of > + * the sink nodes. Since 2 OLDI ports are present in the DT, it > + * can be safely assumed that the required configuration is > + * Clone Mode. > + */ > + return OLDI_SINGLE_LINK_CLONE_MODE; > + > + case DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS: > + /* > + * Note that the OLDI TX 0 transmits the odd set of pixels while > + * the OLDI TX 1 transmits the even set. This is a fixed > + * configuration in the IP and an cannot be change vis SW. These > + * properties have been used to merely identify if a Dual Link > + * configuration is required. Swapping this property in the panel > + * port DT nodes will not make any difference. > + */ > + pr_warn("EVEN-ODD config for dual-link sinks is not supported in HW. Switching to ODD-EVEN.\n"); Please use dev_warn() instead, so that it will be clear where the print comes from. In any case, isn't this an error? Do you really want to accept the wrong pixel order? > + fallthrough; > + > + case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS: > + return OLDI_DUAL_LINK_MODE; > + > + default: > + return OLDI_MODE_OFF; When do we get here? Shouldn't this be OLDI_MODE_UNSUPPORTED? > + } > +} > + > static int tidss_dispc_modeset_init(struct tidss_device *tidss) > { > struct device *dev = tidss->dev; > unsigned int fourccs_len; > const u32 *fourccs = dispc_plane_formats(tidss->dispc, &fourccs_len); > - unsigned int i; > + unsigned int i, j; > > struct pipe { > u32 hw_videoport; > - struct drm_bridge *bridge; > + struct drm_bridge *bridge[TIDSS_MAX_BRIDGES_PER_PIPE]; > u32 enc_type; > + u32 num_bridges; > }; > > const struct dispc_features *feat = tidss->feat; > - u32 max_vps = feat->num_vps; > + u32 output_ports = feat->num_output_ports; > u32 max_planes = feat->num_planes; > > - struct pipe pipes[TIDSS_MAX_VPS]; > + struct pipe pipes[TIDSS_MAX_VPS] = {0}; > + > u32 num_pipes = 0; > u32 crtc_mask; > + u32 portnum_oldi0 = 0, portnum_oldi1 = 2; These two are bit of hacks. First, they should be const, or maybe defines. If they're const, they can be inside the block below. And they're very much tied to the HW in question, so just having hardcoded values here inside a function without any mention of the situation is not good. Doing this in a generic and future proof manner is... challenging. So I think using the hardcoded port numbers is fine. But they are only ok for the two AM6xx SoCs we have now so the 'oldi_supported' is not very good fit. In fact, it might be good to drop 'oldi_supported' altogether, and just check for the SoC versions instead, as (with a quick glance), all the 'oldi_supported' checks are really SoC specific. This also again brings up a thing that rubs me the wrong way: the new OLDI port is port 2. I really think that on AM62x, the two OLDI ports should be 0 and 1, and the DPI should be 2. Would we need a new dt-binding doc for that, or could it still be described in the same doc? Would that change cause changes elsewhere in the dss driver? > + enum dispc_oldi_modes oldi_mode = OLDI_MODE_OFF; > + u32 num_oldi = 0; > + u32 oldi_pipe_index = 0; > + u32 num_encoders = 0; > + > + if (feat->oldi_supported) { > + struct device_node *oldi0_port, *oldi1_port; > + > + oldi0_port = of_graph_get_port_by_id(dev->of_node, > + portnum_oldi0); > + oldi1_port = of_graph_get_port_by_id(dev->of_node, > + portnum_oldi1); > + > + oldi_mode = tidss_get_oldi_mode(oldi0_port, oldi1_port); > + > + of_node_put(oldi0_port); > + of_node_put(oldi1_port); > + > + dispc_set_oldi_mode(tidss->dispc, oldi_mode); > + } > > /* first find all the connected panels & bridges */ > > - for (i = 0; i < max_vps; i++) { > + for (i = 0; i < output_ports; i++) { > struct drm_panel *panel; > struct drm_bridge *bridge; > u32 enc_type = DRM_MODE_ENCODER_NONE; > @@ -145,16 +230,24 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) > return ret; > } > > + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && > + oldi_mode == OLDI_MODE_UNSUPPORTED) { > + dev_err(dev, > + "Single Mode over OLDI 1 is not supported in HW. Keeping OLDI off.\n"); > + continue; Should we error out here? > + } > + > if (panel) { > u32 conn_type; > > dev_dbg(dev, "Setting up panel for port %d\n", i); > > - switch (feat->vp_bus_type[i]) { > + switch (feat->output_port_bus_type[i]) { > case DISPC_VP_OLDI: > enc_type = DRM_MODE_ENCODER_LVDS; > conn_type = DRM_MODE_CONNECTOR_LVDS; > break; > + > case DISPC_VP_DPI: > enc_type = DRM_MODE_ENCODER_DPI; > conn_type = DRM_MODE_CONNECTOR_DPI; > @@ -172,6 +265,17 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) > return -EINVAL; > } > > + /* > + * If the 2nd OLDI port is discovered connected to a panel > + * which is not to be connected in the Clone Mode then a > + * bridge is not required because the detected port is the > + * 2nd port for the previously connected panel. > + */ > + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && > + oldi_mode != OLDI_SINGLE_LINK_CLONE_MODE && Hmm, shouldn't this be oldi_mode == OLDI_DUAL_LINK_MODE? Or rather, shouldn't we test here if this is the second oldi display, and if so which oldi-mode are we using. If dual-link, break. If clone, continue. Otherwise, error. > + num_oldi) > + break; > + > bridge = devm_drm_panel_bridge_add(dev, panel); > if (IS_ERR(bridge)) { > dev_err(dev, > @@ -181,10 +285,47 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) > } > } > > - pipes[num_pipes].hw_videoport = i; > - pipes[num_pipes].bridge = bridge; > - pipes[num_pipes].enc_type = enc_type; > - num_pipes++; > + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI) { > + if (++num_oldi == 1) { > + /* Setting up pipe parameters when 1st OLDI port is detected */ > + > + pipes[num_pipes].hw_videoport = i; > + pipes[num_pipes].enc_type = enc_type; > + > + /* > + * Saving the pipe index in case its required for > + * 2nd OLDI Port. > + */ > + oldi_pipe_index = num_pipes; > + > + /* > + * No additional pipe is required for the 2nd OLDI > + * port, because the 2nd Encoder -> Bridge connection > + * is the part of the first OLDI Port pipe. > + * > + * num_pipes will only be incremented when the first > + * OLDI port is discovered. > + */ > + num_pipes++; > + } > + > + /* > + * Bridge is required to be added only if the detected > + * port is the first OLDI port or a subsequent port in > + * Clone Mode. > + */ > + if (oldi_mode == OLDI_SINGLE_LINK_CLONE_MODE || > + num_oldi == 1) { > + pipes[oldi_pipe_index].bridge[num_oldi - 1] = bridge; > + pipes[oldi_pipe_index].num_bridges++; > + } > + } else { > + pipes[num_pipes].hw_videoport = i; > + pipes[num_pipes].bridge[0] = bridge; > + pipes[num_pipes].num_bridges++; > + pipes[num_pipes].enc_type = enc_type; > + num_pipes++; > + } > } > > /* all planes can be on any crtc */ > @@ -196,6 +337,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) > struct tidss_plane *tplane; > struct tidss_crtc *tcrtc; > struct drm_encoder *enc; > + u32 possible_clones = 0; > u32 hw_plane_id = feat->vid_order[tidss->num_planes]; > int ret; > > @@ -218,16 +360,24 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) > > tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc; > > - enc = tidss_encoder_create(tidss, pipes[i].enc_type, > - 1 << tcrtc->crtc.index); > - if (IS_ERR(enc)) { > - dev_err(tidss->dev, "encoder create failed\n"); > - return PTR_ERR(enc); > - } > + for (j = 0; j < pipes[i].num_bridges; j++) { > + if (pipes[i].num_bridges > 1) > + possible_clones = (((1 << pipes[i].num_bridges) - 1) > + << num_encoders); I think the above possible_clones assignment can be outside the for loop. > + > + enc = tidss_encoder_create(tidss, pipes[i].enc_type, > + 1 << tcrtc->crtc.index, > + possible_clones); > + if (IS_ERR(enc)) { > + dev_err(tidss->dev, "encoder create failed\n"); > + return PTR_ERR(enc); > + } > > - ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0); > - if (ret) > - return ret; > + ret = drm_bridge_attach(enc, pipes[i].bridge[j], NULL, 0); > + if (ret) > + return ret; > + } > + num_encoders += pipes[i].num_bridges; > } > > /* create overlay planes of the leftover planes */ Tomi
Hi Tomi, Thanks for reviewing the patch series. I have implemented the most of your suggestions, but for the others, I needed to clarify things. I have made some comments there. On 20-Dec-22 18:22, Tomi Valkeinen wrote: > Hi, > > On 19/11/2022 19:30, Aradhya Bhatia wrote: >> The newer version of DSS (AM625-DSS) has 2 OLDI TXes at its disposal. >> These can be configured to support the following modes: >> >> 1. OLDI_SINGLE_LINK_SINGLE_MODE >> Single Output over OLDI 0. >> +------+ +---------+ +-------+ >> | | | | | | >> | CRTC +------->+ ENCODER +----->| PANEL | >> | | | | | | >> +------+ +---------+ +-------+ >> >> 2. OLDI_SINGLE_LINK_CLONE_MODE >> Duplicate Output over OLDI 0 and 1. >> +------+ +---------+ +-------+ >> | | | | | | >> | CRTC +---+--->| ENCODER +----->| PANEL | >> | | | | | | | >> +------+ | +---------+ +-------+ >> | >> | +---------+ +-------+ >> | | | | | >> +--->| ENCODER +----->| PANEL | >> | | | | >> +---------+ +-------+ >> >> 3. OLDI_DUAL_LINK_MODE >> Combined Output over OLDI 0 and 1. >> +------+ +---------+ +-------+ >> | | | +----->| | >> | CRTC +------->+ ENCODER | | PANEL | >> | | | +----->| | >> +------+ +---------+ +-------+ >> >> Following the above pathways for different modes, 2 encoder/panel-bridge >> pipes get created for clone mode, and 1 pipe in cases of single link and >> dual link mode. >> >> Add support for confguring the OLDI modes using OF and LVDS DRM helper >> functions. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> drivers/gpu/drm/tidss/tidss_dispc.c | 12 ++ >> drivers/gpu/drm/tidss/tidss_dispc.h | 9 ++ >> drivers/gpu/drm/tidss/tidss_drv.h | 3 + >> drivers/gpu/drm/tidss/tidss_encoder.c | 4 +- >> drivers/gpu/drm/tidss/tidss_encoder.h | 3 +- >> drivers/gpu/drm/tidss/tidss_kms.c | 188 +++++++++++++++++++++++--- >> 6 files changed, 198 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c >> b/drivers/gpu/drm/tidss/tidss_dispc.c >> index dbc6a5b617ca..472226a83251 100644 >> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >> @@ -365,6 +365,8 @@ struct dispc_device { >> struct dss_vp_data vp_data[TIDSS_MAX_VPS]; >> + enum dispc_oldi_modes oldi_mode; >> + >> u32 *fourccs; >> u32 num_fourccs; >> @@ -1967,6 +1969,16 @@ const u32 *dispc_plane_formats(struct >> dispc_device *dispc, unsigned int *len) >> return dispc->fourccs; >> } >> +int dispc_set_oldi_mode(struct dispc_device *dispc, >> + enum dispc_oldi_modes oldi_mode) >> +{ >> + WARN_ON(!dispc); > > This feels unnecessary. Is there even a theoretical case where we could > get dispc == NULL? > >> + >> + dispc->oldi_mode = oldi_mode; >> + >> + return 0; > > This function could as well be void function. > >> +} >> + >> static s32 pixinc(int pixels, u8 ps) >> { >> if (pixels == 1) >> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h >> b/drivers/gpu/drm/tidss/tidss_dispc.h >> index 51db500590ee..e76a7599b544 100644 >> --- a/drivers/gpu/drm/tidss/tidss_dispc.h >> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h >> @@ -64,6 +64,14 @@ enum dispc_dss_subrevision { >> DISPC_AM625, >> }; >> +enum dispc_oldi_modes { >> + OLDI_MODE_OFF, /* OLDI turned off / tied off in IP. */ >> + OLDI_SINGLE_LINK_SINGLE_MODE, /* Single Output over OLDI 0. */ >> + OLDI_SINGLE_LINK_CLONE_MODE, /* Duplicate Output over OLDI 0 and 1. */ >> + OLDI_DUAL_LINK_MODE, /* Combined Output over OLDI 0 and 1. */ >> + OLDI_MODE_UNSUPPORTED, /* Unsupported OLDI Mode */ >> +}; > > What is the difference with MODE_OFF and MODE_UNSUPPORTED? Is > MODE_UNSUPPORTED for cases where, e.g., the DT setup is wrong and the > driver should return an error? The code doesn't quite do that, it prints > an error but then continues. Yes, OLDI_MODE_UNSUPPORTED is for the cases where DT setup is wrong. I have not exited in such a cases with an error, because then the driver will never have a chance to setup the 2nd pipeline (DPI) even if all the DT requirements are met. > >> struct dispc_features { >> int min_pclk_khz; >> int max_pclk_khz[DISPC_VP_MAX_BUS_TYPE]; >> @@ -133,6 +141,7 @@ int dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane, >> u32 hw_videoport); >> int dispc_plane_enable(struct dispc_device *dispc, u32 hw_plane, bool enable); >> const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len); >> +int dispc_set_oldi_mode(struct dispc_device *dispc, enum dispc_oldi_modes oldi_mode); >> int dispc_init(struct tidss_device *tidss); >> void dispc_remove(struct tidss_device *tidss); >> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h >> b/drivers/gpu/drm/tidss/tidss_drv.h >> index 0ce7ee5ccd5b..58892f065c16 100644 >> --- a/drivers/gpu/drm/tidss/tidss_drv.h >> +++ b/drivers/gpu/drm/tidss/tidss_drv.h >> @@ -13,6 +13,9 @@ >> #define TIDSS_MAX_PLANES 4 >> #define TIDSS_MAX_OUTPUT_PORTS 4 >> +/* For AM625-DSS with 2 OLDI TXes */ >> +#define TIDSS_MAX_BRIDGES_PER_PIPE 2 >> + >> typedef u32 dispc_irq_t; >> struct tidss_device { >> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c >> b/drivers/gpu/drm/tidss/tidss_encoder.c >> index e278a9c89476..141383ec4045 100644 >> --- a/drivers/gpu/drm/tidss/tidss_encoder.c >> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c >> @@ -70,7 +70,8 @@ static const struct drm_encoder_funcs encoder_funcs = { >> }; >> struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, >> - u32 encoder_type, u32 possible_crtcs) >> + u32 encoder_type, u32 possible_crtcs, >> + u32 possible_clones) >> { >> struct drm_encoder *enc; >> int ret; >> @@ -80,6 +81,7 @@ struct drm_encoder *tidss_encoder_create(struct >> tidss_device *tidss, >> return ERR_PTR(-ENOMEM); >> enc->possible_crtcs = possible_crtcs; >> + enc->possible_clones = possible_clones; >> ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs, >> encoder_type, NULL); >> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h >> b/drivers/gpu/drm/tidss/tidss_encoder.h >> index ace877c0e0fd..01c62ba3ef16 100644 >> --- a/drivers/gpu/drm/tidss/tidss_encoder.h >> +++ b/drivers/gpu/drm/tidss/tidss_encoder.h >> @@ -12,6 +12,7 @@ >> struct tidss_device; >> struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, >> - u32 encoder_type, u32 possible_crtcs); >> + u32 encoder_type, u32 possible_crtcs, >> + u32 possible_clones); >> #endif >> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c >> b/drivers/gpu/drm/tidss/tidss_kms.c >> index fc9c4eefd31d..8ae321f02197 100644 >> --- a/drivers/gpu/drm/tidss/tidss_kms.c >> +++ b/drivers/gpu/drm/tidss/tidss_kms.c >> @@ -106,30 +106,115 @@ static const struct drm_mode_config_funcs >> mode_config_funcs = { >> .atomic_commit = drm_atomic_helper_commit, >> }; >> +static enum dispc_oldi_modes tidss_get_oldi_mode(struct device_node >> *oldi0_port, >> + struct device_node *oldi1_port) >> +{ >> + int pixel_order; >> + >> + if (!(oldi0_port || oldi1_port)) { >> + /* Keep OLDI TXes off if neither OLDI port is present. */ >> + return OLDI_MODE_OFF; >> + } else if (oldi0_port && !oldi1_port) { >> + /* >> + * OLDI0 port found, but not OLDI1 port. Setting single >> + * link, single mode output. >> + */ >> + return OLDI_SINGLE_LINK_SINGLE_MODE; >> + } else if (!oldi0_port && oldi1_port) { >> + /* >> + * The 2nd OLDI TX cannot be operated alone. This use case is >> + * not supported in the HW. Since the pins for OLDIs 0 and 1 are >> + * separate, one could theoretically set a clone mode over OLDIs >> + * 0 and 1 and just simply not use the OLDI 0. This is a hacky >> + * way to enable only OLDI TX 1 and hence is not officially >> + * supported. >> + */ >> + return OLDI_MODE_UNSUPPORTED; > > If OLDI_MODE_UNSUPPORTED is supposed to result in an error, maybe you > could print the error here (and possibly in the default case below), and > then, in the caller, just return with an error code. > >> + } >> + >> + /* >> + * OLDI Ports found for both the OLDI TXes. The DSS is to be configured >> + * in either Dual Link or Clone Mode. >> + */ >> + pixel_order = drm_of_lvds_get_dual_link_pixel_order(oldi0_port, >> + oldi1_port); >> + switch (pixel_order) { >> + case -EINVAL: >> + /* >> + * The dual link properties were not found in at least one of >> + * the sink nodes. Since 2 OLDI ports are present in the DT, it >> + * can be safely assumed that the required configuration is >> + * Clone Mode. >> + */ >> + return OLDI_SINGLE_LINK_CLONE_MODE; >> + >> + case DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS: >> + /* >> + * Note that the OLDI TX 0 transmits the odd set of pixels while >> + * the OLDI TX 1 transmits the even set. This is a fixed >> + * configuration in the IP and an cannot be change vis SW. These >> + * properties have been used to merely identify if a Dual Link >> + * configuration is required. Swapping this property in the panel >> + * port DT nodes will not make any difference. >> + */ >> + pr_warn("EVEN-ODD config for dual-link sinks is not supported in HW. Switching to ODD-EVEN.\n"); > > Please use dev_warn() instead, so that it will be clear where the print > comes from. > > In any case, isn't this an error? Do you really want to accept the wrong > pixel order? > >> + fallthrough; >> + >> + case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS: >> + return OLDI_DUAL_LINK_MODE; >> + >> + default: >> + return OLDI_MODE_OFF; > > When do we get here? Shouldn't this be OLDI_MODE_UNSUPPORTED? > >> + } >> +} >> + >> static int tidss_dispc_modeset_init(struct tidss_device *tidss) >> { >> struct device *dev = tidss->dev; >> unsigned int fourccs_len; >> const u32 *fourccs = dispc_plane_formats(tidss->dispc, >> &fourccs_len); >> - unsigned int i; >> + unsigned int i, j; >> struct pipe { >> u32 hw_videoport; >> - struct drm_bridge *bridge; >> + struct drm_bridge *bridge[TIDSS_MAX_BRIDGES_PER_PIPE]; >> u32 enc_type; >> + u32 num_bridges; >> }; >> const struct dispc_features *feat = tidss->feat; >> - u32 max_vps = feat->num_vps; >> + u32 output_ports = feat->num_output_ports; >> u32 max_planes = feat->num_planes; >> - struct pipe pipes[TIDSS_MAX_VPS]; >> + struct pipe pipes[TIDSS_MAX_VPS] = {0}; >> + >> u32 num_pipes = 0; >> u32 crtc_mask; >> + u32 portnum_oldi0 = 0, portnum_oldi1 = 2; > > These two are bit of hacks. First, they should be const, or maybe > defines. If they're const, they can be inside the block below. > > And they're very much tied to the HW in question, so just having > hardcoded values here inside a function without any mention of the > situation is not good. > > Doing this in a generic and future proof manner is... challenging. So I > think using the hardcoded port numbers is fine. But they are only ok for > the two AM6xx SoCs we have now so the 'oldi_supported' is not very good > fit. In fact, it might be good to drop 'oldi_supported' altogether, and > just check for the SoC versions instead, as (with a quick glance), all > the 'oldi_supported' checks are really SoC specific. > I will be make these portnum variables const as you suggested and moving these as well as get-node and put-node function calls to the get_oldi_mode function to keep things clear. However, I believe the 'oldi_supported' variable should still be kept (after renaming it to has_oldi as per your suggestion in the previous patch) because having this variable will help distinguish from the cases where an SoC *can* support OLDI output but its output by-passes the OLDI TXes and a DPI output is expected. > This also again brings up a thing that rubs me the wrong way: the new > OLDI port is port 2. I really think that on AM62x, the two OLDI ports > should be 0 and 1, and the DPI should be 2. Would we need a new > dt-binding doc for that, or could it still be described in the same doc? > Would that change cause changes elsewhere in the dss driver? > >> + enum dispc_oldi_modes oldi_mode = OLDI_MODE_OFF; >> + u32 num_oldi = 0; >> + u32 oldi_pipe_index = 0; >> + u32 num_encoders = 0; >> + >> + if (feat->oldi_supported) { >> + struct device_node *oldi0_port, *oldi1_port; >> + >> + oldi0_port = of_graph_get_port_by_id(dev->of_node, >> + portnum_oldi0); >> + oldi1_port = of_graph_get_port_by_id(dev->of_node, >> + portnum_oldi1); >> + >> + oldi_mode = tidss_get_oldi_mode(oldi0_port, oldi1_port); >> + >> + of_node_put(oldi0_port); >> + of_node_put(oldi1_port); >> + >> + dispc_set_oldi_mode(tidss->dispc, oldi_mode); >> + } >> /* first find all the connected panels & bridges */ >> - for (i = 0; i < max_vps; i++) { >> + for (i = 0; i < output_ports; i++) { >> struct drm_panel *panel; >> struct drm_bridge *bridge; >> u32 enc_type = DRM_MODE_ENCODER_NONE; >> @@ -145,16 +230,24 @@ static int tidss_dispc_modeset_init(struct >> tidss_device *tidss) >> return ret; >> } >> + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && >> + oldi_mode == OLDI_MODE_UNSUPPORTED) { >> + dev_err(dev, >> + "Single Mode over OLDI 1 is not supported in HW. Keeping OLDI off.\n"); >> + continue; > > Should we error out here? > >> + } >> + >> if (panel) { >> u32 conn_type; >> dev_dbg(dev, "Setting up panel for port %d\n", i); >> - switch (feat->vp_bus_type[i]) { >> + switch (feat->output_port_bus_type[i]) { >> case DISPC_VP_OLDI: >> enc_type = DRM_MODE_ENCODER_LVDS; >> conn_type = DRM_MODE_CONNECTOR_LVDS; >> break; >> + >> case DISPC_VP_DPI: >> enc_type = DRM_MODE_ENCODER_DPI; >> conn_type = DRM_MODE_CONNECTOR_DPI; >> @@ -172,6 +265,17 @@ static int tidss_dispc_modeset_init(struct >> tidss_device *tidss) >> return -EINVAL; >> } >> + /* >> + * If the 2nd OLDI port is discovered connected to a panel >> + * which is not to be connected in the Clone Mode then a >> + * bridge is not required because the detected port is the >> + * 2nd port for the previously connected panel. >> + */ >> + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && >> + oldi_mode != OLDI_SINGLE_LINK_CLONE_MODE && > > Hmm, shouldn't this be oldi_mode == OLDI_DUAL_LINK_MODE? Or rather, Yes. I will make the change... > shouldn't we test here if this is the second oldi display, and if so > which oldi-mode are we using. If dual-link, break. If clone, continue. > Otherwise, error. but, if I implement the oldi-mode-find logic over here, that would be limited to panels and will skip out the bridges. And the mode-find should really be only done once. That said, I see that this can get a little confusing, So, while keeping the mode-find logic separate, I have organized the other OLDI specific things separately in the next patch. > >> + num_oldi) >> + break; >> + >> bridge = devm_drm_panel_bridge_add(dev, panel); >> if (IS_ERR(bridge)) { >> dev_err(dev, >> @@ -181,10 +285,47 @@ static int tidss_dispc_modeset_init(struct >> tidss_device *tidss) >> } >> } >> - pipes[num_pipes].hw_videoport = i; >> - pipes[num_pipes].bridge = bridge; >> - pipes[num_pipes].enc_type = enc_type; >> - num_pipes++; >> + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI) { >> + if (++num_oldi == 1) { >> + /* Setting up pipe parameters when 1st OLDI port is detected */ >> + >> + pipes[num_pipes].hw_videoport = i; >> + pipes[num_pipes].enc_type = enc_type; >> + >> + /* >> + * Saving the pipe index in case its required for >> + * 2nd OLDI Port. >> + */ >> + oldi_pipe_index = num_pipes; >> + >> + /* >> + * No additional pipe is required for the 2nd OLDI >> + * port, because the 2nd Encoder -> Bridge connection >> + * is the part of the first OLDI Port pipe. >> + * >> + * num_pipes will only be incremented when the first >> + * OLDI port is discovered. >> + */ >> + num_pipes++; >> + } >> + >> + /* >> + * Bridge is required to be added only if the detected >> + * port is the first OLDI port or a subsequent port in >> + * Clone Mode. >> + */ >> + if (oldi_mode == OLDI_SINGLE_LINK_CLONE_MODE || >> + num_oldi == 1) { >> + pipes[oldi_pipe_index].bridge[num_oldi - 1] = bridge; >> + pipes[oldi_pipe_index].num_bridges++; >> + } >> + } else { >> + pipes[num_pipes].hw_videoport = i; >> + pipes[num_pipes].bridge[0] = bridge; >> + pipes[num_pipes].num_bridges++; >> + pipes[num_pipes].enc_type = enc_type; >> + num_pipes++; >> + } >> } >> /* all planes can be on any crtc */ >> @@ -196,6 +337,7 @@ static int tidss_dispc_modeset_init(struct >> tidss_device *tidss) >> struct tidss_plane *tplane; >> struct tidss_crtc *tcrtc; >> struct drm_encoder *enc; >> + u32 possible_clones = 0; >> u32 hw_plane_id = feat->vid_order[tidss->num_planes]; >> int ret; >> @@ -218,16 +360,24 @@ static int tidss_dispc_modeset_init(struct >> tidss_device *tidss) >> tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc; >> - enc = tidss_encoder_create(tidss, pipes[i].enc_type, >> - 1 << tcrtc->crtc.index); >> - if (IS_ERR(enc)) { >> - dev_err(tidss->dev, "encoder create failed\n"); >> - return PTR_ERR(enc); >> - } >> + for (j = 0; j < pipes[i].num_bridges; j++) { >> + if (pipes[i].num_bridges > 1) >> + possible_clones = (((1 << pipes[i].num_bridges) - 1) >> + << num_encoders); > > I think the above possible_clones assignment can be outside the for loop. > >> + >> + enc = tidss_encoder_create(tidss, pipes[i].enc_type, >> + 1 << tcrtc->crtc.index, >> + possible_clones); >> + if (IS_ERR(enc)) { >> + dev_err(tidss->dev, "encoder create failed\n"); >> + return PTR_ERR(enc); >> + } >> - ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0); >> - if (ret) >> - return ret; >> + ret = drm_bridge_attach(enc, pipes[i].bridge[j], NULL, 0); >> + if (ret) >> + return ret; >> + } >> + num_encoders += pipes[i].num_bridges; >> } >> /* create overlay planes of the leftover planes */ Regards Aradhya
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c index dbc6a5b617ca..472226a83251 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.c +++ b/drivers/gpu/drm/tidss/tidss_dispc.c @@ -365,6 +365,8 @@ struct dispc_device { struct dss_vp_data vp_data[TIDSS_MAX_VPS]; + enum dispc_oldi_modes oldi_mode; + u32 *fourccs; u32 num_fourccs; @@ -1967,6 +1969,16 @@ const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len) return dispc->fourccs; } +int dispc_set_oldi_mode(struct dispc_device *dispc, + enum dispc_oldi_modes oldi_mode) +{ + WARN_ON(!dispc); + + dispc->oldi_mode = oldi_mode; + + return 0; +} + static s32 pixinc(int pixels, u8 ps) { if (pixels == 1) diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h index 51db500590ee..e76a7599b544 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.h +++ b/drivers/gpu/drm/tidss/tidss_dispc.h @@ -64,6 +64,14 @@ enum dispc_dss_subrevision { DISPC_AM625, }; +enum dispc_oldi_modes { + OLDI_MODE_OFF, /* OLDI turned off / tied off in IP. */ + OLDI_SINGLE_LINK_SINGLE_MODE, /* Single Output over OLDI 0. */ + OLDI_SINGLE_LINK_CLONE_MODE, /* Duplicate Output over OLDI 0 and 1. */ + OLDI_DUAL_LINK_MODE, /* Combined Output over OLDI 0 and 1. */ + OLDI_MODE_UNSUPPORTED, /* Unsupported OLDI Mode */ +}; + struct dispc_features { int min_pclk_khz; int max_pclk_khz[DISPC_VP_MAX_BUS_TYPE]; @@ -133,6 +141,7 @@ int dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane, u32 hw_videoport); int dispc_plane_enable(struct dispc_device *dispc, u32 hw_plane, bool enable); const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len); +int dispc_set_oldi_mode(struct dispc_device *dispc, enum dispc_oldi_modes oldi_mode); int dispc_init(struct tidss_device *tidss); void dispc_remove(struct tidss_device *tidss); diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h index 0ce7ee5ccd5b..58892f065c16 100644 --- a/drivers/gpu/drm/tidss/tidss_drv.h +++ b/drivers/gpu/drm/tidss/tidss_drv.h @@ -13,6 +13,9 @@ #define TIDSS_MAX_PLANES 4 #define TIDSS_MAX_OUTPUT_PORTS 4 +/* For AM625-DSS with 2 OLDI TXes */ +#define TIDSS_MAX_BRIDGES_PER_PIPE 2 + typedef u32 dispc_irq_t; struct tidss_device { diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c index e278a9c89476..141383ec4045 100644 --- a/drivers/gpu/drm/tidss/tidss_encoder.c +++ b/drivers/gpu/drm/tidss/tidss_encoder.c @@ -70,7 +70,8 @@ static const struct drm_encoder_funcs encoder_funcs = { }; struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, - u32 encoder_type, u32 possible_crtcs) + u32 encoder_type, u32 possible_crtcs, + u32 possible_clones) { struct drm_encoder *enc; int ret; @@ -80,6 +81,7 @@ struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, return ERR_PTR(-ENOMEM); enc->possible_crtcs = possible_crtcs; + enc->possible_clones = possible_clones; ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs, encoder_type, NULL); diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h index ace877c0e0fd..01c62ba3ef16 100644 --- a/drivers/gpu/drm/tidss/tidss_encoder.h +++ b/drivers/gpu/drm/tidss/tidss_encoder.h @@ -12,6 +12,7 @@ struct tidss_device; struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss, - u32 encoder_type, u32 possible_crtcs); + u32 encoder_type, u32 possible_crtcs, + u32 possible_clones); #endif diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c index fc9c4eefd31d..8ae321f02197 100644 --- a/drivers/gpu/drm/tidss/tidss_kms.c +++ b/drivers/gpu/drm/tidss/tidss_kms.c @@ -106,30 +106,115 @@ static const struct drm_mode_config_funcs mode_config_funcs = { .atomic_commit = drm_atomic_helper_commit, }; +static enum dispc_oldi_modes tidss_get_oldi_mode(struct device_node *oldi0_port, + struct device_node *oldi1_port) +{ + int pixel_order; + + if (!(oldi0_port || oldi1_port)) { + /* Keep OLDI TXes off if neither OLDI port is present. */ + return OLDI_MODE_OFF; + } else if (oldi0_port && !oldi1_port) { + /* + * OLDI0 port found, but not OLDI1 port. Setting single + * link, single mode output. + */ + return OLDI_SINGLE_LINK_SINGLE_MODE; + } else if (!oldi0_port && oldi1_port) { + /* + * The 2nd OLDI TX cannot be operated alone. This use case is + * not supported in the HW. Since the pins for OLDIs 0 and 1 are + * separate, one could theoretically set a clone mode over OLDIs + * 0 and 1 and just simply not use the OLDI 0. This is a hacky + * way to enable only OLDI TX 1 and hence is not officially + * supported. + */ + return OLDI_MODE_UNSUPPORTED; + } + + /* + * OLDI Ports found for both the OLDI TXes. The DSS is to be configured + * in either Dual Link or Clone Mode. + */ + pixel_order = drm_of_lvds_get_dual_link_pixel_order(oldi0_port, + oldi1_port); + switch (pixel_order) { + case -EINVAL: + /* + * The dual link properties were not found in at least one of + * the sink nodes. Since 2 OLDI ports are present in the DT, it + * can be safely assumed that the required configuration is + * Clone Mode. + */ + return OLDI_SINGLE_LINK_CLONE_MODE; + + case DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS: + /* + * Note that the OLDI TX 0 transmits the odd set of pixels while + * the OLDI TX 1 transmits the even set. This is a fixed + * configuration in the IP and an cannot be change vis SW. These + * properties have been used to merely identify if a Dual Link + * configuration is required. Swapping this property in the panel + * port DT nodes will not make any difference. + */ + pr_warn("EVEN-ODD config for dual-link sinks is not supported in HW. Switching to ODD-EVEN.\n"); + fallthrough; + + case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS: + return OLDI_DUAL_LINK_MODE; + + default: + return OLDI_MODE_OFF; + } +} + static int tidss_dispc_modeset_init(struct tidss_device *tidss) { struct device *dev = tidss->dev; unsigned int fourccs_len; const u32 *fourccs = dispc_plane_formats(tidss->dispc, &fourccs_len); - unsigned int i; + unsigned int i, j; struct pipe { u32 hw_videoport; - struct drm_bridge *bridge; + struct drm_bridge *bridge[TIDSS_MAX_BRIDGES_PER_PIPE]; u32 enc_type; + u32 num_bridges; }; const struct dispc_features *feat = tidss->feat; - u32 max_vps = feat->num_vps; + u32 output_ports = feat->num_output_ports; u32 max_planes = feat->num_planes; - struct pipe pipes[TIDSS_MAX_VPS]; + struct pipe pipes[TIDSS_MAX_VPS] = {0}; + u32 num_pipes = 0; u32 crtc_mask; + u32 portnum_oldi0 = 0, portnum_oldi1 = 2; + enum dispc_oldi_modes oldi_mode = OLDI_MODE_OFF; + u32 num_oldi = 0; + u32 oldi_pipe_index = 0; + u32 num_encoders = 0; + + if (feat->oldi_supported) { + struct device_node *oldi0_port, *oldi1_port; + + oldi0_port = of_graph_get_port_by_id(dev->of_node, + portnum_oldi0); + oldi1_port = of_graph_get_port_by_id(dev->of_node, + portnum_oldi1); + + oldi_mode = tidss_get_oldi_mode(oldi0_port, oldi1_port); + + of_node_put(oldi0_port); + of_node_put(oldi1_port); + + dispc_set_oldi_mode(tidss->dispc, oldi_mode); + } /* first find all the connected panels & bridges */ - for (i = 0; i < max_vps; i++) { + for (i = 0; i < output_ports; i++) { struct drm_panel *panel; struct drm_bridge *bridge; u32 enc_type = DRM_MODE_ENCODER_NONE; @@ -145,16 +230,24 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) return ret; } + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && + oldi_mode == OLDI_MODE_UNSUPPORTED) { + dev_err(dev, + "Single Mode over OLDI 1 is not supported in HW. Keeping OLDI off.\n"); + continue; + } + if (panel) { u32 conn_type; dev_dbg(dev, "Setting up panel for port %d\n", i); - switch (feat->vp_bus_type[i]) { + switch (feat->output_port_bus_type[i]) { case DISPC_VP_OLDI: enc_type = DRM_MODE_ENCODER_LVDS; conn_type = DRM_MODE_CONNECTOR_LVDS; break; + case DISPC_VP_DPI: enc_type = DRM_MODE_ENCODER_DPI; conn_type = DRM_MODE_CONNECTOR_DPI; @@ -172,6 +265,17 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) return -EINVAL; } + /* + * If the 2nd OLDI port is discovered connected to a panel + * which is not to be connected in the Clone Mode then a + * bridge is not required because the detected port is the + * 2nd port for the previously connected panel. + */ + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI && + oldi_mode != OLDI_SINGLE_LINK_CLONE_MODE && + num_oldi) + break; + bridge = devm_drm_panel_bridge_add(dev, panel); if (IS_ERR(bridge)) { dev_err(dev, @@ -181,10 +285,47 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) } } - pipes[num_pipes].hw_videoport = i; - pipes[num_pipes].bridge = bridge; - pipes[num_pipes].enc_type = enc_type; - num_pipes++; + if (feat->output_port_bus_type[i] == DISPC_VP_OLDI) { + if (++num_oldi == 1) { + /* Setting up pipe parameters when 1st OLDI port is detected */ + + pipes[num_pipes].hw_videoport = i; + pipes[num_pipes].enc_type = enc_type; + + /* + * Saving the pipe index in case its required for + * 2nd OLDI Port. + */ + oldi_pipe_index = num_pipes; + + /* + * No additional pipe is required for the 2nd OLDI + * port, because the 2nd Encoder -> Bridge connection + * is the part of the first OLDI Port pipe. + * + * num_pipes will only be incremented when the first + * OLDI port is discovered. + */ + num_pipes++; + } + + /* + * Bridge is required to be added only if the detected + * port is the first OLDI port or a subsequent port in + * Clone Mode. + */ + if (oldi_mode == OLDI_SINGLE_LINK_CLONE_MODE || + num_oldi == 1) { + pipes[oldi_pipe_index].bridge[num_oldi - 1] = bridge; + pipes[oldi_pipe_index].num_bridges++; + } + } else { + pipes[num_pipes].hw_videoport = i; + pipes[num_pipes].bridge[0] = bridge; + pipes[num_pipes].num_bridges++; + pipes[num_pipes].enc_type = enc_type; + num_pipes++; + } } /* all planes can be on any crtc */ @@ -196,6 +337,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) struct tidss_plane *tplane; struct tidss_crtc *tcrtc; struct drm_encoder *enc; + u32 possible_clones = 0; u32 hw_plane_id = feat->vid_order[tidss->num_planes]; int ret; @@ -218,16 +360,24 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc; - enc = tidss_encoder_create(tidss, pipes[i].enc_type, - 1 << tcrtc->crtc.index); - if (IS_ERR(enc)) { - dev_err(tidss->dev, "encoder create failed\n"); - return PTR_ERR(enc); - } + for (j = 0; j < pipes[i].num_bridges; j++) { + if (pipes[i].num_bridges > 1) + possible_clones = (((1 << pipes[i].num_bridges) - 1) + << num_encoders); + + enc = tidss_encoder_create(tidss, pipes[i].enc_type, + 1 << tcrtc->crtc.index, + possible_clones); + if (IS_ERR(enc)) { + dev_err(tidss->dev, "encoder create failed\n"); + return PTR_ERR(enc); + } - ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0); - if (ret) - return ret; + ret = drm_bridge_attach(enc, pipes[i].bridge[j], NULL, 0); + if (ret) + return ret; + } + num_encoders += pipes[i].num_bridges; } /* create overlay planes of the leftover planes */
The newer version of DSS (AM625-DSS) has 2 OLDI TXes at its disposal. These can be configured to support the following modes: 1. OLDI_SINGLE_LINK_SINGLE_MODE Single Output over OLDI 0. +------+ +---------+ +-------+ | | | | | | | CRTC +------->+ ENCODER +----->| PANEL | | | | | | | +------+ +---------+ +-------+ 2. OLDI_SINGLE_LINK_CLONE_MODE Duplicate Output over OLDI 0 and 1. +------+ +---------+ +-------+ | | | | | | | CRTC +---+--->| ENCODER +----->| PANEL | | | | | | | | +------+ | +---------+ +-------+ | | +---------+ +-------+ | | | | | +--->| ENCODER +----->| PANEL | | | | | +---------+ +-------+ 3. OLDI_DUAL_LINK_MODE Combined Output over OLDI 0 and 1. +------+ +---------+ +-------+ | | | +----->| | | CRTC +------->+ ENCODER | | PANEL | | | | +----->| | +------+ +---------+ +-------+ Following the above pathways for different modes, 2 encoder/panel-bridge pipes get created for clone mode, and 1 pipe in cases of single link and dual link mode. Add support for confguring the OLDI modes using OF and LVDS DRM helper functions. Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> --- drivers/gpu/drm/tidss/tidss_dispc.c | 12 ++ drivers/gpu/drm/tidss/tidss_dispc.h | 9 ++ drivers/gpu/drm/tidss/tidss_drv.h | 3 + drivers/gpu/drm/tidss/tidss_encoder.c | 4 +- drivers/gpu/drm/tidss/tidss_encoder.h | 3 +- drivers/gpu/drm/tidss/tidss_kms.c | 188 +++++++++++++++++++++++--- 6 files changed, 198 insertions(+), 21 deletions(-)