| Message ID | 1419250324-11743-2-git-send-email-alim.akhtar@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Alim, On Mon, Dec 22, 2014 at 4:12 AM, Alim Akhtar <alim.akhtar@samsung.com> wrote: > From: Seungwon Jeon <tgih.jun@samsung.com> > > Even though 1MB is reserved for descriptor table in IDMAC, > the dw_mmc host driver is allowed to receive only maximum > 128KB block length in one request. This is caused by setting > improper max_blk_count. It needs to be e adjusted so that > descriptor table is used fully. It is found that the performance > is improved with the increased the max_blk_count. > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > Acked-by: Jaehoon Chung <jh80.chung@samsung.com> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > drivers/mmc/host/dw_mmc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 64ea042..a1b80e5 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -2332,9 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) > #ifdef CONFIG_MMC_DW_IDMAC > mmc->max_segs = host->ring_size; > mmc->max_blk_size = 65536; > - mmc->max_blk_count = host->ring_size; > mmc->max_seg_size = 0x1000; > - mmc->max_req_size = mmc->max_seg_size * mmc->max_blk_count; > + mmc->max_req_size = mmc->max_seg_size * host->ring_size; > + mmc->max_blk_count = mmc->max_req_size / 512; The numbers are a little different than the ones you and Sonny arrived at about a year ago at <https://chromium-review.googlesource.com/#/c/176488/> You end up with the same max_req_size I think, but it looks as if max_blk_count is 0x800 in your series and ix 0xffff in the one from a year ago. Do you have any explanation for why it is max_req_size / 512? -Doug
Hi Doug / Sonny New year greetings!! On Sat, Jan 3, 2015 at 4:37 AM, Doug Anderson <dianders@chromium.org> wrote: > Alim, > > On Mon, Dec 22, 2014 at 4:12 AM, Alim Akhtar <alim.akhtar@samsung.com> wrote: >> From: Seungwon Jeon <tgih.jun@samsung.com> >> >> Even though 1MB is reserved for descriptor table in IDMAC, >> the dw_mmc host driver is allowed to receive only maximum >> 128KB block length in one request. This is caused by setting >> improper max_blk_count. It needs to be e adjusted so that >> descriptor table is used fully. It is found that the performance >> is improved with the increased the max_blk_count. >> >> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >> Acked-by: Jaehoon Chung <jh80.chung@samsung.com> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> drivers/mmc/host/dw_mmc.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 64ea042..a1b80e5 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -2332,9 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) >> #ifdef CONFIG_MMC_DW_IDMAC >> mmc->max_segs = host->ring_size; >> mmc->max_blk_size = 65536; >> - mmc->max_blk_count = host->ring_size; >> mmc->max_seg_size = 0x1000; >> - mmc->max_req_size = mmc->max_seg_size * mmc->max_blk_count; >> + mmc->max_req_size = mmc->max_seg_size * host->ring_size; >> + mmc->max_blk_count = mmc->max_req_size / 512; > > The numbers are a little different than the ones you and Sonny arrived > at about a year ago at > <https://chromium-review.googlesource.com/#/c/176488/> > > You end up with the same max_req_size I think, but it looks as if > max_blk_count is 0x800 in your series and ix 0xffff in the one from a > year ago. Do you have any explanation for why it is max_req_size / > 512? > Before taking this patch, I went over all the discussions that had happened in past http://marc.info/?l=linux-mmc&m=133039478518090&w=2 (way back in 2012) http://www.spinics.net/lists/linux-mmc/msg25549.html and http://www.spinics.net/lists/linux-mmc/msg25797.html and the last one was having an ACK from maintainer. Having said that, I did check the performance number with current patch which has landed and with one which sets max_blk_count as 0xFFFF. And both given exactly the same numbers on peach-pi (Write: ~52MB/s and Read: ~146MB/s (HS200) using iozone), I checked it a bit more and found that in both the case max_hw_sectors_kb is set to 1024 (max request size to block layer) and max_hw_sectors_kb is the one which actually affect the Performance numbers and max_hw_sectors_kb is set as blk_queue_max_hw_sectors(mq->queue, min(host->max_blk_count, host->max_req_size / 512)); In case of 64bit IDMAC (one on exynos7) which given a different value for "ring_size", both the patches set the same max_hw_sectors_kb (512 in this case) Do you see some performance degradation with some other variants of dw_mmc host controller with current ToT? Sonny, I ran iozone like: iozone -az -i0 -i1 -I -s 10M -f /home/test (on eMMC) > -Doug
On Sat, Jan 3, 2015 at 4:51 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote: > Hi Doug / Sonny > New year greetings!! > > On Sat, Jan 3, 2015 at 4:37 AM, Doug Anderson <dianders@chromium.org> wrote: >> Alim, >> >> On Mon, Dec 22, 2014 at 4:12 AM, Alim Akhtar <alim.akhtar@samsung.com> wrote: >>> From: Seungwon Jeon <tgih.jun@samsung.com> >>> >>> Even though 1MB is reserved for descriptor table in IDMAC, >>> the dw_mmc host driver is allowed to receive only maximum >>> 128KB block length in one request. This is caused by setting >>> improper max_blk_count. It needs to be e adjusted so that >>> descriptor table is used fully. It is found that the performance >>> is improved with the increased the max_blk_count. >>> >>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com> >>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >>> --- >>> drivers/mmc/host/dw_mmc.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>> index 64ea042..a1b80e5 100644 >>> --- a/drivers/mmc/host/dw_mmc.c >>> +++ b/drivers/mmc/host/dw_mmc.c >>> @@ -2332,9 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) >>> #ifdef CONFIG_MMC_DW_IDMAC >>> mmc->max_segs = host->ring_size; >>> mmc->max_blk_size = 65536; >>> - mmc->max_blk_count = host->ring_size; >>> mmc->max_seg_size = 0x1000; >>> - mmc->max_req_size = mmc->max_seg_size * mmc->max_blk_count; >>> + mmc->max_req_size = mmc->max_seg_size * host->ring_size; >>> + mmc->max_blk_count = mmc->max_req_size / 512; >> >> The numbers are a little different than the ones you and Sonny arrived >> at about a year ago at >> <https://chromium-review.googlesource.com/#/c/176488/> >> >> You end up with the same max_req_size I think, but it looks as if >> max_blk_count is 0x800 in your series and ix 0xffff in the one from a >> year ago. Do you have any explanation for why it is max_req_size / >> 512? >> > Before taking this patch, I went over all the discussions that had > happened in past > http://marc.info/?l=linux-mmc&m=133039478518090&w=2 (way back in 2012) > http://www.spinics.net/lists/linux-mmc/msg25549.html > and > http://www.spinics.net/lists/linux-mmc/msg25797.html > > and the last one was having an ACK from maintainer. > > Having said that, I did check the performance number with current > patch which has landed and with one which sets max_blk_count as > 0xFFFF. > And both given exactly the same numbers on peach-pi (Write: ~52MB/s > and Read: ~146MB/s (HS200) using iozone), > > I checked it a bit more and found that in both the case > max_hw_sectors_kb is set to 1024 (max request size to block layer) and > max_hw_sectors_kb is the one which actually affect the Performance > numbers and max_hw_sectors_kb is set as > > blk_queue_max_hw_sectors(mq->queue, min(host->max_blk_count, > host->max_req_size / 512)); > > In case of 64bit IDMAC (one on exynos7) which given a different value > for "ring_size", both the patches set the same max_hw_sectors_kb (512 > in this case) > > Do you see some performance degradation with some other variants of > dw_mmc host controller with current ToT? Looks like Doug test it and saw a performance improvement on rk3288 Thanks! > > Sonny, > I ran iozone like: iozone -az -i0 -i1 -I -s 10M -f /home/test (on eMMC) > >> -Doug > > > > -- > Regards, > Alim
Hi, On Mon, Jan 5, 2015 at 6:27 PM, Sonny Rao <sonnyrao@chromium.org> wrote: >> Do you see some performance degradation with some other variants of >> dw_mmc host controller with current ToT? > > Looks like Doug test it and saw a performance improvement on rk3288 > Thanks! Yup, all good. I tested with mmc_test before and after this patch and saw improvement. I didn't test the old patch. ...only reason I commented originally was because I inspected the patch and compared it to what we had before and saw that they were different. If you've though through all the conditions then I'm happy. -Doug
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 64ea042..a1b80e5 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2332,9 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) #ifdef CONFIG_MMC_DW_IDMAC mmc->max_segs = host->ring_size; mmc->max_blk_size = 65536; - mmc->max_blk_count = host->ring_size; mmc->max_seg_size = 0x1000; - mmc->max_req_size = mmc->max_seg_size * mmc->max_blk_count; + mmc->max_req_size = mmc->max_seg_size * host->ring_size; + mmc->max_blk_count = mmc->max_req_size / 512; #else mmc->max_segs = 64; mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */