Messages in this thread | | | From | Milind Parab <> | Subject | RE: [PATCH v9 2/3] drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge | Date | Thu, 3 Sep 2020 07:32:13 +0000 |
| |
Hi Tomi,
>-----Original Message----- >From: Tomi Valkeinen <tomi.valkeinen@ti.com> >Sent: Thursday, September 3, 2020 12:54 PM >To: Milind Parab <mparab@cadence.com>; Swapnil Kashinath Jakhade ><sjakhade@cadence.com>; airlied@linux.ie; daniel@ffwll.ch; >Laurent.pinchart@ideasonboard.com; robh+dt@kernel.org; >a.hajda@samsung.com; narmstrong@baylibre.com; jonas@kwiboo.se; >jernej.skrabec@siol.net; dri-devel@lists.freedesktop.org; >devicetree@vger.kernel.org; linux-kernel@vger.kernel.org >Cc: Yuti Suresh Amonkar <yamonkar@cadence.com>; jsarha@ti.com; >nsekhar@ti.com; praneeth@ti.com; nikhil.nd@ti.com >Subject: Re: [PATCH v9 2/3] drm: bridge: Add support for Cadence MHDP8546 >DPI/DP bridge > >EXTERNAL MAIL > > >Hi Milind, > >On 03/09/2020 09:22, Milind Parab wrote: > >> Also, note that CDNS MHDP implements DP_FRAMER_TU_p where bits 5:0 >is tu_valid_symbols. So max programmable value is 63. >> Register document gives following explanation "Number of valid symbols >> per Transfer Unit (TU). Rounded down to lower integer value (Allowed >values are 1 to (TU_size-1)" >> >> So, it says in case vs calculates to 64 (where Avail BW and Req BW are >> same) we program tu_valid_symbols = 63 > >Hmm, so "Rounded down to lower integer value" means > >floor(x) - 1 ? > >If that's the case, we need to subtract 1 in all cases, not only when req bw == >avail bw. > Explicit subtraction by 1 is not mentioned anywhere There is a hint of sub-optimal performance when vs equals 64. However need to confirm from simulations.
>> Third, is about the line_threshold calculation Unlike TU_SIZE and >> Valid_Symbols, line_threshold is implementation dependent >> >> CDNS MHDP register specs gives the definition as " Video FIFO latency >threshold" >> Bits 5:0, Name "cfg_active_line_tresh", Description "Video Fifo Latency >threshold. Defines the number of FIFO rows before reading starts. This setting >depends on the transmitted video format and link rate." >> >> This parameter is the Threshold of the FIFO. For optimal performance >(considering equal write and read clock) we normally put the threshold in the >mid of the FIFO. >> Hence the reset value is fixed as 32. >> Since symbol FIFO is accessed by Pxl clock and Symbol Link Clock the >> Threshold is set to a value which is dependent on the ratio of these >> clocks >> >> line_threshold = full_fifo - fifo_ratio_due_to_clock_diff + 2 where, >> full_fifo = (vs+1) * (8/bpp) fifo_ratio_due_to_clock_diff = ((vs+1) * >> pxlclock/mhdp->link.rate - 1) / mhdp->link.num_lanes >> >> Note that line_threshold can take a max value of 63 > >That doesn't result in anything sensible. 8/bpp is always 0.
Consider above statements as pseudocode. For integer division we need scale up like it was scaled by 5 bits in the original code And here 8/bpp = 1 > > Tomi > >-- >Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
| |