On Wed, 12 Sep 2018, Madhav Chauhan <madhav.chauhan@xxxxxxxxx> wrote: > On 9/12/2018 12:56 AM, Jani Nikula wrote: >> On Fri, 20 Jul 2018, "Chauhan, Madhav" <madhav.chauhan@xxxxxxxxx> wrote: >>>> -----Original Message----- >>>> From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >>>> Sent: Thursday, July 19, 2018 9:51 PM >>>> To: Chauhan, Madhav <madhav.chauhan@xxxxxxxxx> >>>> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx>; >>>> Zanoni, Paulo R <paulo.r.zanoni@xxxxxxxxx>; Vivi, Rodrigo >>>> <rodrigo.vivi@xxxxxxxxx> >>>> Subject: Re: [PATCH v5 09/13] drm/i915/icl: Program >>>> TA_TIMING_PARAM registers >>>> >>>> On Tue, Jul 10, 2018 at 03:10:10PM +0530, Madhav Chauhan wrote: >>>>> This patch programs D-PHY timing parameters for the bus turn around >>>>> flow(in escape clocks) only if dsi link frequency <=800 MHz using >>>>> DPHY_TA_TIMING_PARAM and its identical register >>>> DSI_TA_TIMING_PARAM >>>>> (inside DSI Controller within the Display Core). >>>>> >>>>> Signed-off-by: Madhav Chauhan <madhav.chauhan@xxxxxxxxx> >>>>> --- >>>>> drivers/gpu/drm/i915/icl_dsi.c | 21 +++++++++++++++++++++ >>>>> drivers/gpu/drm/i915/intel_dsi.h | 1 + >>>>> drivers/gpu/drm/i915/intel_dsi_vbt.c | 1 + >>>>> 3 files changed, 23 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/icl_dsi.c >>>>> b/drivers/gpu/drm/i915/icl_dsi.c index 832772d..8fd5284 100644 >>>>> --- a/drivers/gpu/drm/i915/icl_dsi.c >>>>> +++ b/drivers/gpu/drm/i915/icl_dsi.c >>>>> @@ -302,6 +302,27 @@ static void gen11_dsi_setup_dphy_timings(struct >>>> intel_encoder *encoder) >>>>> I915_WRITE(DSI_DATA_TIMING_PARAM(port), >>>>> intel_dsi->dphy_data_lane_reg); >>>>> } >>>>> + >>>>> + /* >>>>> + * If DSI link operating at or below an 800 MHz, >>>>> + * TA_SURE should be override and programmed to >>>>> + * a value '0' inside TA_PARAM_REGISTERS otherwise >>>>> + * leave all fields at HW default values. >>>>> + */ >>>>> + if (intel_dsi->bitrate_khz <= KHz(800)) { >>>> The KHz(800) confuses me. My brain thinks this value is 800 kHz when it's >>>> not. So I'd write it without the KHz() macro. >>> Ok. Initially I wrote without using KHz macro, but got comment to use KHz macro :) >> Did I? Oh well. Go with 800000. > > Ok :) > >> >> Please don't add additional state with intel_dsi->bitrate_khz when you >> can calculate the bitrate at any time. Add a function to do it if you >> like, and use it in both places. > > Ok. But we will reusing this bitrate(same value) in multiple place. > Shouldn't we cache it rather than calculating everytime?? Depends. The performance impact is neglible. Caching has its own problems, but here I suppose you don't need to invalidate it. In the end, I think it boils down to code readability. In this case, I guess go with what you have then. BR, Jani. > > Regards, > Madhav > >> >> BR, >> Jani. >> >> >>> Regards, >>> Madhav >>> >>>>> + for_each_dsi_port(port, intel_dsi->ports) { >>>>> + tmp = I915_READ(DPHY_TA_TIMING_PARAM(port)); >>>>> + tmp &= ~TA_SURE_TIME_MASK; >>>>> + tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0)); >>>>> + I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp); >>>>> + >>>>> + /* shadow register inside display core */ >>>>> + tmp = I915_READ(DSI_TA_TIMING_PARAM(port)); >>>>> + tmp &= ~TA_SURE_TIME_MASK; >>>>> + tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0)); >>>>> + I915_WRITE(DSI_TA_TIMING_PARAM(port), tmp); >>>>> + } >>>>> + } >>>>> } >>>>> >>>>> static void gen11_dsi_enable_port_and_phy(struct intel_encoder >>>>> *encoder) diff --git a/drivers/gpu/drm/i915/intel_dsi.h >>>>> b/drivers/gpu/drm/i915/intel_dsi.h >>>>> index 9fd8526..25e7396 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_dsi.h >>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.h >>>>> @@ -101,6 +101,7 @@ struct intel_dsi { >>>>> >>>>> u16 init_count; >>>>> u32 pclk; >>>>> + u32 bitrate_khz; >>>>> u16 burst_mode_ratio; >>>>> >>>>> /* all delays in ms */ >>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c >>>>> b/drivers/gpu/drm/i915/intel_dsi_vbt.c >>>>> index 428290d..a9a98a4 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c >>>>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c >>>>> @@ -589,6 +589,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, >>>> u16 panel_id) >>>>> intel_dsi->pclk = pclk; >>>>> >>>>> bitrate = (pclk * bpp) / intel_dsi->lane_count; >>>>> + intel_dsi->bitrate_khz = bitrate; >>>>> >>>>> switch (intel_dsi->escape_clk_div) { >>>>> case 0: >>>>> -- >>>>> 2.7.4 >>>>> >>>>> _______________________________________________ >>>>> Intel-gfx mailing list >>>>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>>> -- >>>> Ville Syrjälä >>>> Intel > -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
