On Tue, 24 Nov 2020 at 01:58, Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx> wrote:
>
> Hi Loic,
> On 2020-11-23 06:11 AM, Loic Poulain wrote:
> > Hardware channels have a feature called burst mode that allows to
> > queue transfer ring element(s) (TRE) to a channel without ringing
> > the device doorbell. In that mode, the device is polling the channel
> > context for new elements. This reduces the frequency of host initiated
> > doorbells and increase throughput.
> >
> > Create a new dedicated macro for hardware channels with burst enabled.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
> > ---
> > drivers/bus/mhi/pci_generic.c | 34 ++++++++++++++++++++++++++++++++--
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bus/mhi/pci_generic.c
> > b/drivers/bus/mhi/pci_generic.c
> > index 09c6b26..0c07cf5 100644
> > --- a/drivers/bus/mhi/pci_generic.c
> > +++ b/drivers/bus/mhi/pci_generic.c
> > @@ -78,6 +78,36 @@ struct mhi_pci_dev_info {
> > .offload_channel = false, \
> > }
> >
> > +#define MHI_CHANNEL_CONFIG_HW_UL(ch_num, ch_name, el_count, ev_ring) \
> > + { \
> > + .num = ch_num, \
> > + .name = ch_name, \
> > + .num_elements = el_count, \
> > + .event_ring = ev_ring, \
> > + .dir = DMA_TO_DEVICE, \
> > + .ee_mask = BIT(MHI_EE_AMSS), \
> > + .pollcfg = 0, \
> > + .doorbell = MHI_DB_BRST_ENABLE, \
> > + .lpm_notify = false, \
> > + .offload_channel = false, \
> > + .doorbell_mode_switch = false, \
> > + } \
> > +
> > +#define MHI_CHANNEL_CONFIG_HW_DL(ch_num, ch_name, el_count, ev_ring) \
> > + { \
> > + .num = ch_num, \
> > + .name = ch_name, \
> > + .num_elements = el_count, \
> > + .event_ring = ev_ring, \
> > + .dir = DMA_FROM_DEVICE, \
> > + .ee_mask = BIT(MHI_EE_AMSS), \
> > + .pollcfg = 0, \
> > + .doorbell = MHI_DB_BRST_ENABLE, \
> > + .lpm_notify = false, \
> > + .offload_channel = false, \
> > + .doorbell_mode_switch = false, \
> > + }
> > +
> > #define MHI_EVENT_CONFIG_DATA(ev_ring) \
> > { \
> > .num_elements = 128, \
> > @@ -112,8 +142,8 @@ static const struct mhi_channel_config
> > modem_qcom_v1_mhi_channels[] = {
> > MHI_CHANNEL_CONFIG_DL(15, "QMI", 4, 0),
> > MHI_CHANNEL_CONFIG_UL(20, "IPCR", 8, 0),
> > MHI_CHANNEL_CONFIG_DL(21, "IPCR", 8, 0),
> > - MHI_CHANNEL_CONFIG_UL(100, "IP_HW0", 128, 1),
> > - MHI_CHANNEL_CONFIG_DL(101, "IP_HW0", 128, 2),
> > + MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0", 128, 1),
> > + MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 2),
> > };
> >
> > static const struct mhi_event_config modem_qcom_v1_mhi_events[] = {
> Did you somehow miss my email with comments on this macro from the
> previous
> submission?
>
> If not, then any reason you want to submit this way for now and change
> it
> later when more HW channels are added?
>
> I don't think it's a good idea to have doorbell_mode_switch as false for
> channel 100 as we need to ring the DB every time we come out of M3.
My bad, I missed that point. I'm going to fix that. BTW would it no
make sense to always enabled that from MHI core (and not let the
controller driver to choose)?
>
> The current proposal will become inconsistent when more HW channels with
> different requirements are added so my suggestion was to accommodate
> these
> now. Also, I realized we need to update the regular macros as well but
> it
> can be dealt with separately.
>
> If you would like recommendations or would want to discuss this
> configuration
> thing further, please let me know.
>
> Thanks,
> Bhaumik
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> a Linux Foundation Collaborative Project
