On 01/12/2020 02:36, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Tue, Nov 24, 2020 at 02:45:34PM +0200, Tomi Valkeinen wrote:
>> As we now have a fixed setup for VCs (VC0 for video stream, VC1 for
>> commands), we can simplify the VC setup.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>> ---
>> drivers/gpu/drm/omapdrm/dss/dsi.c | 85 +++++++++++--------------------
>> 1 file changed, 31 insertions(+), 54 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> index ff8ace957291..27d0d119668b 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -2017,40 +2017,6 @@ static void dsi_vc_initial_config(struct dsi_data *dsi, int vc)
>> dsi->vc[vc].source = DSI_VC_SOURCE_L4;
>> }
>>
>> -static int dsi_vc_config_source(struct dsi_data *dsi, int vc,
>> - enum dsi_vc_source source)
>> -{
>> - if (dsi->vc[vc].source == source)
>> - return 0;
>> -
>> - DSSDBG("Source config of VC %d", vc);
>> -
>> - dsi_sync_vc(dsi, vc);
>> -
>> - dsi_vc_enable(dsi, vc, 0);
>> -
>> - /* VC_BUSY */
>> - if (!wait_for_bit_change(dsi, DSI_VC_CTRL(vc), 15, 0)) {
>> - DSSERR("vc(%d) busy when trying to config for VP\n", vc);
>> - return -EIO;
>> - }
>> -
>> - /* SOURCE, 0 = L4, 1 = video port */
>> - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), source, 1, 1);
>> -
>> - /* DCS_CMD_ENABLE */
>> - if (dsi->data->quirks & DSI_QUIRK_DCS_CMD_CONFIG_VC) {
>> - bool enable = source == DSI_VC_SOURCE_VP;
>> - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), enable, 30, 30);
>> - }
>> -
>> - dsi_vc_enable(dsi, vc, 1);
>> -
>> - dsi->vc[vc].source = source;
>> -
>> - return 0;
>> -}
>> -
>> static void dsi_vc_enable_hs(struct omap_dss_device *dssdev, int vc,
>> bool enable)
>> {
>> @@ -2072,10 +2038,6 @@ static void dsi_vc_enable_hs(struct omap_dss_device *dssdev, int vc,
>> dsi_if_enable(dsi, 1);
>>
>> dsi_force_tx_stop_mode_io(dsi);
>> -
>> - /* start the DDR clock by sending a NULL packet */
>> - if (dsi->vm_timings.ddr_clk_always_on && enable)
>> - dsi_vc_send_null(dsi, vc, dsi->dsidev->channel);
>> }
>>
>> static void dsi_vc_flush_long_data(struct dsi_data *dsi, int vc)
>> @@ -2270,8 +2232,6 @@ static int dsi_vc_send_long(struct dsi_data *dsi, int vc,
>> return -EINVAL;
>> }
>>
>> - dsi_vc_config_source(dsi, vc, DSI_VC_SOURCE_L4);
>> -
>> dsi_vc_write_long_header(dsi, vc, msg->channel, msg->type, msg->tx_len, 0);
>>
>> p = msg->tx_buf;
>> @@ -2331,8 +2291,6 @@ static int dsi_vc_send_short(struct dsi_data *dsi, int vc,
>> DSSDBG("dsi_vc_send_short(ch%d, dt %#x, b1 %#x, b2 %#x)\n",
>> vc, msg->type, pkt.header[1], pkt.header[2]);
>>
>> - dsi_vc_config_source(dsi, vc, DSI_VC_SOURCE_L4);
>> -
>> if (FLD_GET(dsi_read_reg(dsi, DSI_VC_CTRL(vc)), 16, 16)) {
>> DSSERR("ERROR FIFO FULL, aborting transfer\n");
>> return -EINVAL;
>> @@ -3351,8 +3309,6 @@ static void dsi_update_screen_dispc(struct dsi_data *dsi)
>>
>> DSSDBG("dsi_update_screen_dispc(%dx%d)\n", w, h);
>>
>> - dsi_vc_config_source(dsi, vc, DSI_VC_SOURCE_VP);
>> -
>> bytespp = mipi_dsi_pixel_format_to_bpp(dsi->pix_fmt) / 8;
>> bytespl = w * bytespp;
>> bytespf = bytespl * h;
>> @@ -3522,9 +3478,7 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
>>
>> dsi_set_ulps_auto(dsi, false);
>>
>> - dsi_vc_enable_hs(dssdev, vc, !(dsi->dsidev->mode_flags & MIPI_DSI_MODE_LPM));
>
> Why is this not needed anymore ?
This is in dsi_update_channel(), so about sending the frame. We always want to send the frame in HS,
and we set the VC to HS in the dsi_setup_dsi_vcs() in this patch. So there's no longer need to
configure the speed for each dsi_update_channel invocation.
So there's a slight change in behavior, as before this patch you could send frames in LP, but can't
after this. But who would want to do that...
I think I can clean this up by changing "drm/omap: dsi: set LP/HS before update" to "always enable
HS before update". And then dropping the line here.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
