On Tue, 24 Nov 2020 at 09:05, Loic Poulain <loic.poulain@xxxxxxxxxx> wrote:
>
> 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)?
And let me know if there is any reason to not enable it
doorbell_mode_switch when burst mode is enabled. I would like macro
paramaeters as minimal as possible so that they simplify channel
configuration.
Regards,
Loic
