Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1571872 > unrolled thread
| Started by | Sean Paul <seanpaul@chromium.org> |
|---|---|
| First post | 2017-02-01 19:20 +0100 |
| Last post | 2017-02-05 08:50 +0100 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Sean Paul <seanpaul@chromium.org> - 2017-02-01 19:20 +0100
Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Chris Zhong <zyw@rock-chips.com> - 2017-02-05 04:50 +0100
Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Mark yao <mark.yao@rock-chips.com> - 2017-02-05 08:50 +0100
| From | Sean Paul <seanpaul@chromium.org> |
|---|---|
| Date | 2017-02-01 19:20 +0100 |
| Subject | Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function |
| Message-ID | <t67FE-SH-15@gated-at.bofh.it> |
On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
> Hi Sean
>
> On 01/24/2017 01:48 AM, Sean Paul wrote:
> >On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
> >>The MIPI DSI do not need check the validity of resolution, the max
> >>resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
> >>here.
> >Does vop actually enforce this, though? I see that mode_config.max_width is
> >4096, but there is no bounds checking in mode_fixup().
> >
> >The connector is currently rejecting everything greater than 2047. So I think
> >you're going to regress behavior here.
> >
> >Sean
> The mipi controller has not this width limit, it depend the VOP,
> such as RK3399, VOP_LIT only support 2560,
> but VOP_BIG support 4K. So this driver should check the width here.
I don't see anything in the vop driver that rejects large modes for little vop.
So, while I agree the check shouldn't be here, you should move it to where it
should be instead of removing it entirely.
Sean
>
> >
> >
> >>Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> >>---
> >>
> >>Changes in v3: None
> >>
> >> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
> >> 1 file changed, 39 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>index a93ce97..6f0e252 100644
> >>--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>@@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
> >> u32 grf_dsi0_mode;
> >> u32 grf_dsi0_mode_reg;
> >> unsigned int max_data_lanes;
> >>- enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> >>- struct drm_display_mode *mode);
> >> };
> >> struct dw_mipi_dsi {
> >>@@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
> >> return drm_panel_get_modes(dsi->panel);
> >> }
> >>-static enum drm_mode_status dw_mipi_dsi_mode_valid(
> >>- struct drm_connector *connector,
> >>- struct drm_display_mode *mode)
> >>-{
> >>- struct dw_mipi_dsi *dsi = con_to_dsi(connector);
> >>-
> >>- enum drm_mode_status mode_status = MODE_OK;
> >>-
> >>- if (dsi->pdata->mode_valid)
> >>- mode_status = dsi->pdata->mode_valid(connector, mode);
> >>-
> >>- return mode_status;
> >>-}
> >>-
> >> static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
> >> .get_modes = dw_mipi_dsi_connector_get_modes,
> >>- .mode_valid = dw_mipi_dsi_mode_valid,
> >> };
> >> static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
> >>@@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
> >> return 0;
> >> }
> >>-static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
> >>- struct drm_connector *connector,
> >>- struct drm_display_mode *mode)
> >>-{
> >>- /*
> >>- * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
> >>- * register is 11-bit.
> >>- */
> >>- if (mode->hdisplay > 0x7ff)
> >>- return MODE_BAD_HVALUE;
> >>-
> >>- /*
> >>- * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
> >>- * register is 11-bit.
> >>- */
> >>- if (mode->vdisplay > 0x7ff)
> >>- return MODE_BAD_VVALUE;
> >>-
> >>- return MODE_OK;
> >>-}
> >>-
> >> static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
> >> .dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
> >> .dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
> >> .grf_switch_reg = RK3288_GRF_SOC_CON6,
> >> .max_data_lanes = 4,
> >>- .mode_valid = rk3288_mipi_dsi_mode_valid,
> >> };
> >> static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> >>--
> >>2.6.3
> >>
> >>_______________________________________________
> >>dri-devel mailing list
> >>dri-devel@lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
[toc] | [next] | [standalone]
| From | Chris Zhong <zyw@rock-chips.com> |
|---|---|
| Date | 2017-02-05 04:50 +0100 |
| Message-ID | <t7lZU-1TQ-5@gated-at.bofh.it> |
| In reply to | #1571872 |
On 02/02/2017 02:12 AM, Sean Paul wrote:
> On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
>> Hi Sean
>>
>> On 01/24/2017 01:48 AM, Sean Paul wrote:
>>> On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
>>>> The MIPI DSI do not need check the validity of resolution, the max
>>>> resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
>>>> here.
>>> Does vop actually enforce this, though? I see that mode_config.max_width is
>>> 4096, but there is no bounds checking in mode_fixup().
>>>
>>> The connector is currently rejecting everything greater than 2047. So I think
>>> you're going to regress behavior here.
>>>
>>> Sean
>> The mipi controller has not this width limit, it depend the VOP,
>> such as RK3399, VOP_LIT only support 2560,
>> but VOP_BIG support 4K. So this driver should check the width here.
> I don't see anything in the vop driver that rejects large modes for little vop.
> So, while I agree the check shouldn't be here, you should move it to where it
> should be instead of removing it entirely.
>
> Sean
drm_mode_validate_size will check the dev->mode_config.max_width and
dev->mode_config.max_height, these 2 value come from
rockchip_drm_mode_config_init,
currently, they are both 4096. So you are right, drm driver does not
distinguish
between vop lit and big.
I think Mark Yao already have a local solution, and he will post it soon.
>
>>>
>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v3: None
>>>>
>>>> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
>>>> 1 file changed, 39 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> index a93ce97..6f0e252 100644
>>>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>>>> u32 grf_dsi0_mode;
>>>> u32 grf_dsi0_mode_reg;
>>>> unsigned int max_data_lanes;
>>>> - enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>>>> - struct drm_display_mode *mode);
>>>> };
>>>> struct dw_mipi_dsi {
>>>> @@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>>>> return drm_panel_get_modes(dsi->panel);
>>>> }
>>>> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
>>>> - struct drm_connector *connector,
>>>> - struct drm_display_mode *mode)
>>>> -{
>>>> - struct dw_mipi_dsi *dsi = con_to_dsi(connector);
>>>> -
>>>> - enum drm_mode_status mode_status = MODE_OK;
>>>> -
>>>> - if (dsi->pdata->mode_valid)
>>>> - mode_status = dsi->pdata->mode_valid(connector, mode);
>>>> -
>>>> - return mode_status;
>>>> -}
>>>> -
>>>> static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
>>>> .get_modes = dw_mipi_dsi_connector_get_modes,
>>>> - .mode_valid = dw_mipi_dsi_mode_valid,
>>>> };
>>>> static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
>>>> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
>>>> return 0;
>>>> }
>>>> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>>>> - struct drm_connector *connector,
>>>> - struct drm_display_mode *mode)
>>>> -{
>>>> - /*
>>>> - * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>>>> - * register is 11-bit.
>>>> - */
>>>> - if (mode->hdisplay > 0x7ff)
>>>> - return MODE_BAD_HVALUE;
>>>> -
>>>> - /*
>>>> - * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>>>> - * register is 11-bit.
>>>> - */
>>>> - if (mode->vdisplay > 0x7ff)
>>>> - return MODE_BAD_VVALUE;
>>>> -
>>>> - return MODE_OK;
>>>> -}
>>>> -
>>>> static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>>>> .dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>>>> .dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>>>> .grf_switch_reg = RK3288_GRF_SOC_CON6,
>>>> .max_data_lanes = 4,
>>>> - .mode_valid = rk3288_mipi_dsi_mode_valid,
>>>> };
>>>> static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
>>>> --
>>>> 2.6.3
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[toc] | [prev] | [next] | [standalone]
| From | Mark yao <mark.yao@rock-chips.com> |
|---|---|
| Date | 2017-02-05 08:50 +0100 |
| Message-ID | <t7pKa-4nz-1@gated-at.bofh.it> |
| In reply to | #1573803 |
On 2017年02月05日 11:42, Chris Zhong wrote:
>
>
> On 02/02/2017 02:12 AM, Sean Paul wrote:
>> On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
>>> Hi Sean
>>>
>>> On 01/24/2017 01:48 AM, Sean Paul wrote:
>>>> On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
>>>>> The MIPI DSI do not need check the validity of resolution, the max
>>>>> resolution should depend VOP. Hence, remove
>>>>> rk3288_mipi_dsi_mode_valid
>>>>> here.
>>>> Does vop actually enforce this, though? I see that
>>>> mode_config.max_width is
>>>> 4096, but there is no bounds checking in mode_fixup().
>>>>
>>>> The connector is currently rejecting everything greater than 2047.
>>>> So I think
>>>> you're going to regress behavior here.
>>>>
>>>> Sean
>>> The mipi controller has not this width limit, it depend the VOP,
>>> such as RK3399, VOP_LIT only support 2560,
>>> but VOP_BIG support 4K. So this driver should check the width here.
>> I don't see anything in the vop driver that rejects large modes for
>> little vop.
>> So, while I agree the check shouldn't be here, you should move it to
>> where it
>> should be instead of removing it entirely.
>>
>> Sean
>
> drm_mode_validate_size will check the dev->mode_config.max_width and
> dev->mode_config.max_height, these 2 value come from
> rockchip_drm_mode_config_init,
> currently, they are both 4096. So you are right, drm driver does not
> distinguish
> between vop lit and big.
>
> I think Mark Yao already have a local solution, and he will post it soon.
Hi
See follow patches, support mode valid with vop callback.
[0] https://patchwork.kernel.org/patch/9555943/
[1] https://patchwork.kernel.org/patch/9555945/
>
>>
>>>>
>>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v3: None
>>>>>
>>>>> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39
>>>>> ----------------------------------
>>>>> 1 file changed, 39 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> index a93ce97..6f0e252 100644
>>>>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>>>>> u32 grf_dsi0_mode;
>>>>> u32 grf_dsi0_mode_reg;
>>>>> unsigned int max_data_lanes;
>>>>> - enum drm_mode_status (*mode_valid)(struct drm_connector
>>>>> *connector,
>>>>> - struct drm_display_mode *mode);
>>>>> };
>>>>> struct dw_mipi_dsi {
>>>>> @@ -1081,23 +1079,8 @@ static int
>>>>> dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>>>>> return drm_panel_get_modes(dsi->panel);
>>>>> }
>>>>> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
>>>>> - struct drm_connector *connector,
>>>>> - struct drm_display_mode *mode)
>>>>> -{
>>>>> - struct dw_mipi_dsi *dsi = con_to_dsi(connector);
>>>>> -
>>>>> - enum drm_mode_status mode_status = MODE_OK;
>>>>> -
>>>>> - if (dsi->pdata->mode_valid)
>>>>> - mode_status = dsi->pdata->mode_valid(connector, mode);
>>>>> -
>>>>> - return mode_status;
>>>>> -}
>>>>> -
>>>>> static struct drm_connector_helper_funcs
>>>>> dw_mipi_dsi_connector_helper_funcs = {
>>>>> .get_modes = dw_mipi_dsi_connector_get_modes,
>>>>> - .mode_valid = dw_mipi_dsi_mode_valid,
>>>>> };
>>>>> static void dw_mipi_dsi_drm_connector_destroy(struct
>>>>> drm_connector *connector)
>>>>> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct
>>>>> dw_mipi_dsi *dsi)
>>>>> return 0;
>>>>> }
>>>>> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>>>>> - struct drm_connector *connector,
>>>>> - struct drm_display_mode *mode)
>>>>> -{
>>>>> - /*
>>>>> - * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>>>>> - * register is 11-bit.
>>>>> - */
>>>>> - if (mode->hdisplay > 0x7ff)
>>>>> - return MODE_BAD_HVALUE;
>>>>> -
>>>>> - /*
>>>>> - * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>>>>> - * register is 11-bit.
>>>>> - */
>>>>> - if (mode->vdisplay > 0x7ff)
>>>>> - return MODE_BAD_VVALUE;
>>>>> -
>>>>> - return MODE_OK;
>>>>> -}
>>>>> -
>>>>> static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>>>>> .dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>>>>> .dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>>>>> .grf_switch_reg = RK3288_GRF_SOC_CON6,
>>>>> .max_data_lanes = 4,
>>>>> - .mode_valid = rk3288_mipi_dsi_mode_valid,
>>>>> };
>>>>> static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
>>>>> --
>>>>> 2.6.3
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
>
--
Ｍark Yao
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web